-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
style(sqllab): update table count styling #15200
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15200 +/- ##
==========================================
- Coverage 77.40% 77.17% -0.23%
==========================================
Files 969 971 +2
Lines 50013 50291 +278
Branches 6431 6140 -291
==========================================
+ Hits 38712 38814 +102
- Misses 11098 11273 +175
- Partials 203 204 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -78,6 +78,10 @@ const TableLabel = styled.span` | |||
} | |||
`; | |||
|
|||
const smallColor = css` | |||
color: rgba(102, 102, 102, 0.7); |
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.
@lyndsiWilliams do we have anything in supersetTheme with lower opacity?
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.
ah yeah, I think you should format it like this:
https://github.com/apache/superset/blob/pexdax/db-connection-ui/superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts#L46 (above that there is one that uses theme and css).
Instead of grayscale.base try grayscale.light1? (play around with it).
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.
I think colors.text.help will be the next lightest color.
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.
{tableOptions.length} in | ||
<i>{schema}</i> | ||
<small css={smallColor}> | ||
{tableOptions.length} {t(' ')} in {schema} |
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.
you don't need to wrap the space in the translate function t(' ')
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.
actually since they're on the same line now, you should be able to use a normal space without the string literal {' '}.
{tableOptions.length} in {schema}
Thanks @stellalc7 for this fix! |
@yousoph what are your thoughts? |
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 great, thanks you!
Nice, thanks @stellalc7 ! |
* Change text style and opacity * Edit changes to text style and opacity for table length * Edit changes to text style and opacity
* Change text style and opacity * Edit changes to text style and opacity for table length * Edit changes to text style and opacity
* Change text style and opacity * Edit changes to text style and opacity for table length * Edit changes to text style and opacity
SUMMARY
removed italics, and change opacity in table count text
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE
![image](https://user-images.githubusercontent.com/17345270/122276182-85400d00-ceb2-11eb-9daa-be65ab4860ab.png)
AFTER
![Screenshot 2021-06-16 at 14 18 27](https://user-images.githubusercontent.com/17345270/122276180-83764980-ceb2-11eb-8450-2d00775d7da3.png)
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION