Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Modal component #347

Merged
merged 100 commits into from
Jul 31, 2019
Merged

Modal component #347

merged 100 commits into from
Jul 31, 2019

Conversation

coostenbrug
Copy link
Contributor

No description provided.

@coostenbrug coostenbrug force-pushed the Modal-Component branch 2 times, most recently from 06b06ee to 6716a48 Compare July 25, 2019 14:29
matthewlyle
matthewlyle reviewed Jul 26, 2019
View reviewed changes
Copy link
Contributor

@matthewlyle matthewlyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

design and docs look good to me 👍

coostenbrug added 26 commits July 26, 2019 11:30
@coostenbrug
adds Modal files
f40fbaa
@coostenbrug
adds modal markup and scrolling
240c0d1
@coostenbrug
adjust some height values
993f232
@coostenbrug
adds button mapping
55a4d6b
@coostenbrug
adds case where there is no footer
af840d3
@coostenbrug
fixes spacing
961ef25
@coostenbrug
changes strategy for scrolling/responsive to flex
d283e95
@coostenbrug
removes set heights on header/footer
ac47ae6
@coostenbrug
responsive width
fc07bb8
@coostenbrug
adds case when there is no modal title
24de63b
@coostenbrug
brings in React-Modal
d5418bb
@coostenbrug
removed close on an outside click
0ba70ef
@coostenbrug
removes DimPage and restyles overlay
8b7d254
@coostenbrug
resets border style
ee39a7d
@coostenbrug
adds close button to the modal component
95233f8
@coostenbrug
adds extra padding when close button is present
9f75fce
@coostenbrug
corrects centering of the modal on the screen
30af414
@coostenbrug
allows all button props to be passed in
c58b2d8
@coostenbrug
changes ButtonSet styling so that margin is applied on the right of b…
976eaa6 
…uttons
@coostenbrug
makes Modal component controlled only
3c7509b
@coostenbrug
clears some console errors
ee62bb1
@coostenbrug
supports Modal.setAppElement
ca97e28
@coostenbrug
fixes media query on modal width
448b668
@coostenbrug
moved button font override to modal component
e3b0909
@coostenbrug
makes shouldCloseOnOverlayClick a prop set default to true
a3f1b6f
@coostenbrug
removes requirements on buttons
230e0e8
clemensp
clemensp approved these changes Jul 30, 2019
View reviewed changes
Copy link
Contributor

@clemensp clemensp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Left a few comments to consider. :)

components/src/Modal/Modal.js Show resolved Hide resolved
components/src/Modal/Modal.js Outdated Show resolved Hide resolved
components/src/Modal/Modal.js Outdated Show resolved Hide resolved
components/src/Modal/Modal.story.js Show resolved Hide resolved
docs/src/pages/components/navbar.js Outdated Show resolved Hide resolved
@coostenbrug coostenbrug merged commit 55fa50c into master Jul 31, 2019
@marboro92 marboro92 deleted the Modal-Component branch May 15, 2020 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewlyle matthewlyle matthewlyle left review comments

@clemensp clemensp clemensp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@coostenbrug @clemensp @matthewlyle @nikola-nulogy