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

fix(Modal): Fixed bottom padding on the Modal header #1175

Merged
merged 8 commits into from
Jan 5, 2024
Merged

fix(Modal): Fixed bottom padding on the Modal header #1175

merged 8 commits into from
Jan 5, 2024

Conversation

chris-cedrone-cengage
Copy link
Collaborator

@chris-cedrone-cengage chris-cedrone-cengage commented Jan 3, 2024 •
edited
Loading

Issue: # 1173

What I did

Removed bottom padding from Modal header.

Screenshots:

Paddingery

Checklist

  • changeset has been added
  • Pull request description is descriptive
  • [N/A] I have made corresponding changes to the documentation
  • New and existing unit tests pass locally with my changes
  • [N/A] I have added tests that prove my fix is effective or that my feature works

How to test

  • Go to Doc site
  • Open Modal and ensure there is no bottom padding on the Modal header

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jan 3, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 451d8f8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

silvalaura
silvalaura reviewed Jan 3, 2024
View reviewed changes
.changeset/fix-modalPadding.md Outdated
@@ -0,0 +1,5 @@
---
'react-magma-dom': minor
Copy link
Collaborator

Choose a reason for hiding this comment

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

patch

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@chris-cedrone-cengage chris-cedrone-cengage added the needs design feedback Waiting for Design approval or feedback label Jan 3, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@silvalaura
Copy link
Collaborator

A couple things I noticed for discussion:

  1. The example for modal with no header looks odd:
    image
  2. The modal with header with only one paragraph (no margins) also looks odd:
    image
  3. Hide close button, no margin:
    image
  4. In the nested modal example, the second modal's paragraph needs noMargins
    image

I guess the question is, are we okay with no spacing between the button and paragraph in these scenarios?
@orion-cengage

@silvalaura
Copy link
Collaborator

This PR is on hold until #1177 gets merged so we can use the new props here.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 5, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 5, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@silvalaura
Copy link
Collaborator

Just one more thing, I think at standup we talked about adding to storybook some of the similar examples to those in the docs (no header, small, no close button, etc)

@chris-cedrone-cengage
Copy link
Collaborator Author

chris-cedrone-cengage commented Jan 5, 2024 •
edited
Loading

Just one more thing, I think at standup we talked about adding to storybook some of the similar examples to those in the docs (no header, small, no close button, etc)

"No Header Or Focusable Content" Has all three of those versions in one. Should I break them out?

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 5, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 5, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@silvalaura
Copy link
Collaborator

https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

I missed that, but that is good enough for me!

silvalaura
silvalaura approved these changes Jan 5, 2024
View reviewed changes
Copy link
Collaborator

@silvalaura silvalaura left a comment

Choose a reason for hiding this comment

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

Looks good (again!). Just same comment about the PR title :)

@chris-cedrone-cengage chris-cedrone-cengage changed the title Modal Header - Padding Fix fix(Modal): Fixed bottom padding on the Modal header Jan 5, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 5, 2024

🚀 Deployed on https://docs-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 5, 2024

🚀 Deployed on https://storybook-preview-1175--upbeat-sinoussi-f675aa.netlify.app

@chris-cedrone-cengage chris-cedrone-cengage merged commit 421b909 into dev Jan 5, 2024
3 checks passed
chris-cedrone-cengage added a commit that referenced this pull request Jan 19, 2024
@chris-cedrone-cengage
fix(Modal): Fixed bottom padding on the Modal header (#1175)
8ccca96
silvalaura pushed a commit that referenced this pull request Jan 19, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
b57a990
silvalaura pushed a commit that referenced this pull request Jan 22, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
dfbee0b
silvalaura pushed a commit that referenced this pull request Jun 5, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
edccab5
silvalaura pushed a commit that referenced this pull request Jun 18, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
0e2daf2
silvalaura pushed a commit that referenced this pull request Jun 18, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
9c3c2af
silvalaura pushed a commit that referenced this pull request Jun 20, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
6c8ca7d
silvalaura pushed a commit that referenced this pull request Aug 15, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
e50b1b4
silvalaura pushed a commit that referenced this pull request Sep 18, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
b801b58
silvalaura pushed a commit that referenced this pull request Sep 18, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
ad13cde
silvalaura pushed a commit that referenced this pull request Sep 18, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
63b49ba
silvalaura pushed a commit that referenced this pull request Sep 18, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
a603f0d
silvalaura pushed a commit that referenced this pull request Sep 18, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
a64d97f
silvalaura pushed a commit that referenced this pull request Dec 16, 2024
@chris-cedrone-cengage @silvalaura
fix(Modal): Fixed bottom padding on the Modal header (#1175)
509e457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silvalaura silvalaura silvalaura approved these changes

@orion-cengage orion-cengage orion-cengage approved these changes

Assignees
No one assigned
Labels
needs design feedback Waiting for Design approval or feedback
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chris-cedrone-cengage @silvalaura @orion-cengage