-
Notifications
You must be signed in to change notification settings - Fork 0
Add SectionMessage story #9
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
base: main
Are you sure you want to change the base?
Conversation
| .message { | ||
| margin-bottom: 0; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this style is not necessary, because the margin-bottom: 0 is already present for <p> (default element) and inherent for <div> (typical alternative element) (screenshot).
Musing on the current Message components styles…
I've long since thought:
-
Some
marginshould always be present between multiple messages; perhaps:.container + .container:not(p) /* ignore `<p>` cuz it typically has margin */ { margin-top: /* …something, maybe `1rem` */ }
-
The default tag should be
<div>.
So use of<p>by client can rely on whatever<p>margin they set (or allow from browser).
| .container { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 1rem; | ||
| padding: 1rem; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not add these styles to the demo, because:
- The lack of space between messages shows clients what will happen by default if they stack multiple.
- Storybook already adds space to the
<body>in the preview window (via.sb-main-padded). If we start doing it for every.container(or.rootor whatever the element may be classed), then we give ourselves a chore to remain consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this! Seeing a new story is… indescribably satisfying.
I do notice:
- the
scopeoption is not working (screenshot) - the
typeoption is ineffectual
Overview
Add a simple section message storybook story
Related
None
Screenshots
Notes
@wesleyboar , just trying story book out.