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.

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

fix(Alert): padding issue when no dismiss button #2853

Merged
merged 26 commits into from
Apr 8, 2024
Merged

Conversation

LinKCoding
Copy link
Contributor

@LinKCoding LinKCoding commented Apr 2, 2024

Overview

Added responsive right padding for Alerts where there is no close button.
Smaller fixes for other files for consistency.

PR Checklist

Testing Instructions

  1. Boot up the server locally
  2. go to packages/gamut/src/Alert/Alert.tsx
  3. when checking the spacing for no close button, you'll need to go to packages/styleguide/stories/Molecules/Alert/index.stories.mdx and inside an args set onClose: false

E.g.

<Meta
  title={title}
  component={Alert}
  parameters={{
    subtitle:
      'An alert displays an important, succinct message, and provides actions for users to address (or dismiss the alert). It requires a user action to be dismissed.',
    status: 'current',
    source: 'gamut',
    design: {
      type: 'figma',
      url: 'https://www.figma.com/file/TemYhRWOavsERWprDzRJCx',
    },
    padded: true,
  }}
  args={{
    type: 'general',
    placement: 'floating',
    cta: {
      children: 'Learn More',
    },
    children:
      'You have been logged out due to inactivity. Click the button to log in again.',
   // ADDED ONCLOSE HERE
    onClose: false,
  }}
/>

^ you can also go to .constants and change onClose there too.
4. Inspect the element(s) and check that the spacing(padding + column-gap) on the right-hand side is applied and that elements match the design's specs for all breakpoints
5. you may note that for the smallest breakpoint it's set to 4 ({_:4 ...} that's because the column-gap is 12 when there's no buttons, so both padding and column will add up to 16.
6 ...
7. Done!

PR Links and Envs

(None, just Gamut)

@LinKCoding LinKCoding changed the title storing hacky stuff for now, will likely revert (fix) Alert: padding issue when no dismiss button Apr 2, 2024
@LinKCoding LinKCoding changed the title (fix) Alert: padding issue when no dismiss button fix(Alert): padding issue when no dismiss button Apr 4, 2024
@LinKCoding LinKCoding marked this pull request as ready for review April 4, 2024 18:22
@LinKCoding LinKCoding requested a review from a team as a code owner April 4, 2024 18:22
Copy link
Contributor

@dreamwasp dreamwasp left a comment

Choose a reason for hiding this comment

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

have some comments to clean up but otherwise looking great!

@LinKCoding
Copy link
Contributor Author

have some comments to clean up but otherwise looking great!

@dreamwasp should be fixed now, sorry about that!

Copy link
Contributor

@dreamwasp dreamwasp left a comment

Choose a reason for hiding this comment

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

@codecademydev
Copy link
Collaborator

📬Published Alpha Packages:

@codecademy/[email protected]
@codecademy/[email protected]
@codecademy/[email protected]

@codecademydev
Copy link
Collaborator

🚀 Styleguide deploy preview ready!

https://661416e6e949a20fee5029b3--gamut-preview.netlify.app

Deploy Logs

@LinKCoding LinKCoding added the Ship It :shipit: Automerge this PR when possible label Apr 8, 2024
@codecademydev codecademydev merged commit 8232a19 into main Apr 8, 2024
17 of 19 checks passed
@codecademydev codecademydev deleted the kl-gm-608 branch April 8, 2024 18:11
@codecademydev codecademydev removed the Ship It :shipit: Automerge this PR when possible label Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants