-
Notifications
You must be signed in to change notification settings - Fork 18
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
32 - Add buttons and dropdowns of the design system #41
32 - Add buttons and dropdowns of the design system #41
Conversation
Sprint kickoff:
|
I'm confused. I thought we recently decided to use Bootstrap 5 styles without any alteration. Also, I'm nervous about relying on tools that must be run manually. I also don't love the idea of seeing "Serious" in a tool and ignoring it. So perhaps we should restore the original Bootstrap 5 button which, from what I understand, doesn't have a background gradient. |
Hi @pdurbin ! I may have misunderstood the resolution of the Bootstrap styles thing, what I understood is that we were going to use Bootstrap 5 styles without any alteration except for the Bootstrap theme variables, to set the primary and secondary color. Because overriding these variables doesn't add more complexity since it's part of the available Bootstrap variables to customize the theme and it doesn't requite changing the CSS. It's just a bunch of variables to make a clean customization
Then there is a third variable which is Just so we are on the same page, I also changed other theme variables such as the $font-family or $text-color. Is that ok, or do we want to keep everything exactly as plain Bootstrap?
I totally agree! I just let the gradient because it was already in the JSF frontend. I can set the variable to false |
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.
looks good, fixes the Accessibility issue
@MellyGray I'm ready to merge but there is a merge conflict that needs to be resolved. |
…torybook 40 - Add mdx template to use in storybook
@kcondon sorry! I'll try to solve the conflicts next time before moving to QA. Did you solve the conflicts? I can't see them anymore |
…-design-system 30 - Add the design tokens of the design system
…s-of-the-design-system 32 - Add buttons and dropdowns of the design system
…s-of-the-design-system 32 - Add buttons and dropdowns of the design system
…s-of-the-design-system 32 - Add buttons and dropdowns of the design system
What this PR does / why we need it:
This PR adds the buttons and dropdown elements of the Dataverse design system as React components having them documented in the Storybook. This will make it easier for developers to use and maintain consistent buttons and dropdowns across the application.
Which issue(s) this PR closes:
Special notes for your reviewer:
I've used Bootstrap Button and Dropdown encapsulated in our own component.
I've made some customisation to the Bootstrap CSS styles to match the current styles of the Dataverse JSF frontend. This customisation made me add a css property with the
!important
, which is a bad practice, to override a css style added by Bootstrap js.I believe this wouldn't be necessary if we used Bootstrap as it is, just changing the theme variables, instead of making a huge customisation. I've assumed we wanted this customisation so the new frontend looks as the one in JSF as specified in the current Style Guide but let me know if we can stick to Bootstrap styles so I can do the changes to remove the property with the !important
Suggestions on how to test this:
npm run storybook
Go to http://localhost:6006
In the UI section you can see the Button and DropdownButton components with their different states
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
This PR adds the button and dropdown of the Dataverse Design System as specified in the Analysis doc
Is there a release notes update needed for this change?:
Button and dropdown added to the Design System
Additional documentation: