-
Notifications
You must be signed in to change notification settings - Fork 14k
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: Database selector overflow #17369
fix: Database selector overflow #17369
Conversation
@@ -97,8 +107,10 @@ const SelectLabel = ({ | |||
databaseName: string; | |||
}) => ( | |||
<LabelStyle> | |||
<Label>{backend}</Label> | |||
{databaseName} | |||
<Label className="backend">{backend}</Label> |
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.
Would it be better if we created separate styled components instead of adding classes to the container?
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.
Every styled component generates a class underneath. I think this pattern is better for highly correlated components like this example, where all subcomponents are part of an atomic label. Breaking styles into multiple components for atomic units seems worse.
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.
Tested, works perfectly
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.
Lgtm!
Codecov Report
@@ Coverage Diff @@
## master #17369 +/- ##
=======================================
Coverage 77.14% 77.14%
=======================================
Files 1036 1036
Lines 55759 55759
Branches 7628 7628
=======================================
Hits 43013 43013
Misses 12490 12490
Partials 256 256
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
const selection = await screen.findByTitle( | ||
'GMT -06:00 (Mountain Daylight Time)', | ||
); | ||
const selection = await screen.findByTitle('GMT -06:00 (America/Belize)'); |
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.
we should coverage "offsetsToName" in src/components/TimezoneSelector/index.tsx
. I have a seprate PR do this.
🏷️ 2021.44 |
* fix: Database selector overflow * Fix TImezoneSelector test * Removes unused imports (cherry picked from commit ed4a628)
* fix: Database selector overflow * Fix TImezoneSelector test * Removes unused imports
SUMMARY
Fixes the database selector overflow.
@rusackas
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
1 - Go to SQL Editor
2 - Select a database with a long name
3 - Check that the text is overflowed correctly
ADDITIONAL INFORMATION