-
Notifications
You must be signed in to change notification settings - Fork 152
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(ButtonPrimitive): add missing flex-auto #4212
Conversation
9cae8ad
to
911be5f
Compare
Deploying with
|
Latest commit: |
911be5f
|
Status: | ✅ Deploy successful! |
Preview URL: | https://fe4592b7.orbit.pages.dev |
Branch Preview URL: | https://fix-buttonprimitive-fullwidt.orbit.pages.dev |
@@ -51,6 +51,23 @@ BasicButtons.story = { | |||
}, | |||
}; | |||
|
|||
export const MultipleFullWidthButtonsInContainer = () => { |
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.
The story doesn't seem to add much documentation value to the component (and it looks quite ugly in our storybook). We should have a visual test for this.
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.
For sake of simplicity and fixing the bug faster I skipped visual testing here, especially that they are in Chromatic and have to rewritten to Playwright. So maybe keep that story until we will rewrite the visual tests. Wdyt? Tbh, I do not think we should care much about how it looks, if it's ugly or not, it does what it should. Another option is to remove the story, I added that to make sure that it works.
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.
especially that they are in Chromatic and have to rewritten to Playwright. So maybe keep that story until we will rewrite the visual tests.
Alright, agreed.
Tbh, I do not think we should care much about how it looks, if it's ugly or not, it does what it should.
Hmm a story has a purpose of documentation (especially now that we are not relying on stories for visual tests). This story tells nothing to the user. It doesn't showcase any prop or anything in particular…
So, for now, I'm ok with keeping it as is 👍
Size Change: +5 B (0%) Total Size: 460 kB
ℹ️ View Unchanged
|
Reported here
flex: 1 1 auto
was present in older version of Orbit in styled-components, got removed during Tailwind migrationStorybook: https://orbit-mainframev-fix-buttonprimitive-fullwidth.surge.sh