-
Notifications
You must be signed in to change notification settings - Fork 4
feat: remove ul from bass.css file and update components to use list #733
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
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.
Few small things, otherwise its looking good!
| </ul> | ||
| <Components.UnorderedList | ||
| items={[ | ||
| <> |
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.
So a few things, if you look at the UnorderedList component, you'll see it actually provides the LI's. So we just need to provide the text here since the component will render this on each item.
<li key={key} className={styles.item}>
{item}
</li>
)```
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.
let me fix that!
| .list { | ||
| position: relative; | ||
| width: 100%; | ||
| list-style-type: none !important; |
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.
So, we're probably not style linting but we sorta have a no important rule on this team (Which we've written down nowhere).
Can we try to tackle these using the module to scope this? Take a look at how we use .root in other modules to achieve specificity.
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.
ohhh! yes, absolutely.
| className={styles.list} | ||
| items={requirements | ||
| .sort((a, b) => (a.completed ? -1 : 1)) | ||
| .map((step, i) => { |
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.
This looks good cause of how its returning an array w/o LIs :)
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.
yay! :)
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Description
This PR migrates from global
<ul>styles to using the component adapter'sUnorderedListcomponent, improving consistency and customizability across the SDK.JIRA Ticket: https://gustohq.atlassian.net/browse/GWS-5723
Changes Made
1. Removed Global
<ul>Styles (src/styles/_Base.scss)<ul>elements that was applyingmargin: 0,display: flex,flex-direction: column,gap, andpaddingUnorderedListcomponent2. Enhanced
UnorderedListComponent (src/components/Common/UI/List/List.module.scss)display: flex,flex-direction: columngap: toRem(12)padding: 0 0 0 toRem(24)3. Updated
RequirementsListComponentTypeScript (
src/components/Common/RequirementsList/RequirementsList.tsx):<ul>and<li>elements withComponents.UnorderedList<li>to<div>(sinceUnorderedListwraps items in<li>automatically)Styles (
src/components/Common/RequirementsList/RequirementsList.module.scss):list-style-type: none !important,padding: 0 !important,gap: 0 !important.listItem:not(:last-child)::afterto:global(li):not(:last-child) .listItem::afterto accommodate the new HTML structure4. Updated
Employee/LandingComponent (src/components/Employee/Landing/Landing.tsx)<ul>and<li>elements withComponents.UnorderedListitemsprop5. Updated Welcome Stories (
.ladle/Welcome.stories.tsx)Components.UnorderedListTesting
RequirementsListmaintains its visual appearance (numbered icons, checkmarks, dotted connectors)Employee/Landinglist displays correctlyScreenshots