lib/ui utility classes#1147
Merged
RobbieTheWagner merged 9 commits intoemberjs:masterfrom Mar 9, 2020
Merged
Conversation
nummi
force-pushed
the
lib-ui-utility-classes
branch
from
8af82bc to
974fa28
Compare
RobbieTheWagner
requested changes
Feb 22, 2020
Member
RobbieTheWagner
left a comment
RobbieTheWagner
left a comment
There was a problem hiding this comment.
Looks mostly good! Just a few suggestions / comments.
|
|
||
| .text-inherit { color: var(--inherit); } | ||
|
|
||
| .bg-base00 { background-color: var(--base00); } |
Member
There was a problem hiding this comment.
Tailwind supports setting your own colors to be used for both background-color and text color, so these could actually be part of tailwind.
| .text-base11 { color: var(--base11); } | ||
| .text-base13 { color: var(--base13); } | ||
|
|
||
| .text-12 { font-size: 12px; } |
Member
There was a problem hiding this comment.
I'm not sure how many text sizes we have, but maybe we should try to standardize on text-xs text-sm etc to align with tailwind?
Contributor
Author
There was a problem hiding this comment.
I've been thinking about this. Since we have an app the spectrum of font sizes goes something like: font-size: 10px – 15px. Not sure it makes sense to have names like this:
text-xs: 10px
text-sm: 11px
text-base: 12px
text-lg: 13px
text-xl: 14px
We don't need our fonts to range this size:
Member
There was a problem hiding this comment.
I think your suggestion sounds fine:
text-xs: 10px
text-sm: 11px
text-base: 12px
text-lg: 13px
text-xl: 14pxIt is a little bit odd to have them all 1px different though. I think I would advocate more for setting a base font size and making all these rem based like tailwind does.
RobbieTheWagner
approved these changes
Feb 26, 2020
Member
RobbieTheWagner
left a comment
RobbieTheWagner
left a comment
There was a problem hiding this comment.
LGTM! Mind running yarn to get things updated?
nummi
force-pushed
the
lib-ui-utility-classes
branch
from
f14289d to
c2a7979
Compare
- Move to lib/ui - Utility classes
- utility classes
- utility classes
- utility classes
- utility classes
- utility classes
- utility classes
- Use ember-test-selectors across entire app - Upgrade ember-test-selectors
nummi
force-pushed
the
lib-ui-utility-classes
branch
from
c2a7979 to
a66c302
Compare
nummi
added a commit
to nummi/ember-inspector
that referenced
this pull request
Apr 1, 2020
* Cleanup warning message component - Move to lib/ui - Utility classes * Cleanup error page component - utility classes * Cleanup disclosure triangle component - utility classes * Cleanup drag handle component - utility classes * Cleanup empty message component - utility classes * Cleanup toolbar divider component - utility classes * Cleanup toolbar search field component - utility classes * Misc nav and pill css cleanup * Use ember-test-selectors - Use ember-test-selectors across entire app - Upgrade ember-test-selectors
nummi
added a commit
to nummi/ember-inspector
that referenced
this pull request
Apr 5, 2020
* Cleanup warning message component - Move to lib/ui - Utility classes * Cleanup error page component - utility classes * Cleanup disclosure triangle component - utility classes * Cleanup drag handle component - utility classes * Cleanup empty message component - utility classes * Cleanup toolbar divider component - utility classes * Cleanup toolbar search field component - utility classes * Misc nav and pill css cleanup * Use ember-test-selectors - Use ember-test-selectors across entire app - Upgrade ember-test-selectors
patricklx
pushed a commit
to patricklx/ember-inspector
that referenced
this pull request
Sep 19, 2022
* Cleanup warning message component - Move to lib/ui - Utility classes * Cleanup error page component - utility classes * Cleanup disclosure triangle component - utility classes * Cleanup drag handle component - utility classes * Cleanup empty message component - utility classes * Cleanup toolbar divider component - utility classes * Cleanup toolbar search field component - utility classes * Misc nav and pill css cleanup * Use ember-test-selectors - Use ember-test-selectors across entire app - Upgrade ember-test-selectors
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Moving more components to utility classes to help with the amount of CSS we maintain and help with possible move to Tailwind. Most utility classes added follow Tailwind CSS naming — there are unique classes marked at the bottom of
utils.scss.The easiest way to review is probably going through each commit one at a time.