EDSC-4421: Fixing styling discrepancies from bootstrap upgrade #1870
Conversation
Bundle Size ComparisonFull build detailsThe full bundle is larger than main by 0.2 kB. ❗ The index.js is smaller than main by 0 kB. 🎉 Run
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1870 +/- ##
=======================================
Coverage 93.42% 93.42%
=======================================
Files 781 781
Lines 19167 19167
Branches 4970 4969 -1
=======================================
Hits 17906 17906
+ Misses 1206 1176 -30
- Partials 55 85 +30 ☔ View full report in Codecov by Sentry. |
| @@ -26,6 +26,10 @@ | |||
| margin-right: calc(bootstrap.$spacer / 2); | |||
| } | |||
|
|
|||
| .btn-close { | |||
There was a problem hiding this comment.
I don't understand this change. this should be core functionality of react-bootstrap, what are we doing to break it?
There was a problem hiding this comment.
From what I could find, the bootstrap upgrade likely changed how the close button is positioned within the stacking context which caused this issue. Why it's only on this modal, I'm not sure.
There was a problem hiding this comment.
When we upgraded bootstrap, in the EDSCModal we changed from rendering our own close button to using the react-bootstrap closeButton prop. It looks like we already used that prop on ShapefileUploadModal before the upgrade.
react-bootstrap docs specify that onHide is called when the closeButton is clicked, so we are doing something that breaks that base functionality. The fix for that shouldn't be to override the library's styles, we should find what we are doing to break it and fix that.
There was a problem hiding this comment.
I was able to get the facet length issue fixed with only including overflow: hidden; to FacetsItem.scss on &__title-container. I didn't need to change any min/max-width values
I was able to fix the sidebar filter wrapping text with only adding font-size: 0.825rem; to SidebarFiltersItem.scss on &__body
Using &__header is definitely better than using .btn-close in ShapefileUploadModal.scss. The probem/difference with the Shapefile modal and EDSCModal seems to be the &:before css where we are putting a dashed border inset on the modal.
I was able to fix the Subscription list icon alignment by adding the class align-middle to the EDSCIcon call on line 116 of SubscriptionsListTable.jsx
| @@ -13,6 +13,11 @@ | |||
|
|
|||
| &__actions { | |||
| text-align: center; | |||
| div { | |||
There was a problem hiding this comment.
We need to avoid doing nested selectors if we can. If you cant avoid nesting then maybe theres a class you can use instead of an element selector.
There was a problem hiding this comment.
Shouldn't need this at all with the addition of the align-middle bootstrap class in SubscriptionListTable.jsx
I agree with matthew. I noticed that this is using the bootstrap |
| &__header { | ||
| z-index: 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
| &__header { | |
| z-index: 0; | |
| } |
There was a problem hiding this comment.
@dpesall / @eudoroolivares2016, I played around with this and a better way to fix it would be to add the following in the &:before on line 37
&:before {
...
user-select: none;
.is-dragging & {
....
}
}
This will prevent that pseudo element from intercepting the click events 😄
Overview
What is the feature?
Fixing some minor styling errors that resulted from upgrading our version of bootstrap.
What is the Solution?
xbutton on Shape File modalWhat areas of the application does this impact?
Testing
Reproduction steps
Attachments
Checklist