Use display helper consistently for all components #2114
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rajsite
commented
May 15, 2024
rajsite
commented
May 15, 2024
rajsite
commented
May 15, 2024
rajsite
requested review from
jattasNI,
vivinkrishna-ni,
DStavilaNI and
munteannatan
as code owners
2 tasks
rajsite
commented
May 16, 2024
jattasNI
reviewed
May 16, 2024
vivinkrishna-ni
approved these changes
May 16, 2024
rajsite
commented
May 16, 2024
munteannatan
approved these changes
May 16, 2024
jattasNI
approved these changes
May 16, 2024
1 task
rajsite
added a commit
that referenced
this pull request
Jun 10, 2024
# Pull Request ## 🤨 Rationale Switch to using `box-sizing: border-box;` consistently in all our components. ## 👩💻 Implementation - Each component should use the `display` helper when defined which was made consistent in #2114. This PR leverages that helper to make sure each component sets the elements in the shadow root to use `box-sizing: border-box;` consistently - Created separate selectors for targeting all children, i.e. `* {}`, the host itself, i.e. `:host {}`, and all before and after pseduo-elements so devtools stays clearer, specifically the selector for `:host` and the selector for `*` are clear in dev tools without the noise of the `::before` and `::after` selectors - Cleared the manual configuration for `box-sizing` - Cleaned up some padding configurations (now consistently included in the size inside the border) to be margin configurations (which are outside the border and not part of the size). ## 🧪 Testing Rely on chromatic, explanation of some of the changes: - Menu changes look correct, the minimum size now includes the border instead of the border going outside the configured size - Select filter box changes look improved. The sizing of the filter box and the no options box now fit the control height instead of being larger than the control height - Rich text changes look innocuous. Added the interaction design figma we have but don't think there are strict visual design docs to reference. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
1 task
rajsite
added a commit
that referenced
this pull request
Jun 11, 2024
# Pull Request ## 🤨 Rationale Switch to using `box-sizing: border-box;` consistently in all our components. fixes #1326 ## 👩💻 Implementation - Each component should use the `display` helper when defined which was made consistent in #2114. This PR leverages that helper to make sure each component sets the elements in the shadow root to use `box-sizing: border-box;` consistently - Created separate selectors for targeting all children, i.e. `* {}`, the host itself, i.e. `:host {}`, and all before and after pseduo-elements so devtools stays clearer, specifically the selector for `:host` and the selector for `*` are clear in dev tools without the noise of the `::before` and `::after` selectors - Cleared the manual configuration for `box-sizing` - Cleaned up some padding configurations (now consistently included in the size inside the border) to be margin configurations (which are outside the border and not part of the size). ## 🧪 Testing Rely on chromatic, explanation of some of the changes: - Menu changes look correct, the minimum size now includes the border instead of the border going outside the configured size - Select filter box changes look improved. The sizing of the filter box and the no options box now fit the control height instead of being larger than the control height - Rich text changes align with figma (border included in the 40px of the footer section). Added the interaction design figma to the component status table but don't think there are strict visual design docs to reference. - Recreated the PR from squashed commits so that reviewers can see Chromatic Test changes more easily. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
🤨 Rationale
This PR updates all defined components to use a custom version of the
displayhelper. The goal of this PR is to be a refactor to use the same helper but should be effectively a no-op.Part of #1326
👩💻 Implementation
display: none;🧪 Testing
Rely on storybook to verify nothing changed.
✅ Checklist
displaydoc topic.