-
Notifications
You must be signed in to change notification settings - Fork 73
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
[FC-0036] refactor: Further refinements to tag drawer #970
[FC-0036] refactor: Further refinements to tag drawer #970
Conversation
* Padding to top and left tagging drawer * Changes in headings in the tagging drawer
Thanks for the pull request, @ChrisChV! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #970 +/- ##
==========================================
+ Coverage 92.12% 92.21% +0.09%
==========================================
Files 685 703 +18
Lines 12133 12359 +226
Branches 2642 2677 +35
==========================================
+ Hits 11177 11397 +220
- Misses 920 926 +6
Partials 36 36 ☔ View full report in Codecov by Sentry. |
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.
👍 @ChrisChV Looks good to me, great work!
- I tested this: (describe what you tested)
- I read through the code
- I checked for accessibility issues
-
Includes documentation
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.
Just a few minor nits about using existing classes instead of creating new ones. After that this is good to merge.
@@ -9,6 +9,10 @@ | |||
} | |||
|
|||
.tags-drawer { | |||
.tags-drawer-heading { | |||
font-size: 1.375rem !important; |
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 this is the font size of h3, you can use the h3
class name to apply that style.
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.
Updated: 6e959bb
@@ -18,6 +22,14 @@ | |||
background-color: transparent; | |||
color: $gray-300 !important; | |||
} | |||
|
|||
.tags-drawer-subtitle { | |||
font-size: 1.125rem !important; |
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.
Likewise you can use h4
as a class here.
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.
Updated 6e959bb
} | ||
|
||
.tags-drawer-taxonomy-name { | ||
font-size: .875rem !important; |
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.
And this is h5
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.
Updated 6e959bb
@xitij2000 It's ready for another review |
@ChrisChV 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
Supporting information
Testing instructions
Make sure you have some sample taxonomy/tags data setup: https://github.com/open-craft/taxonomy-sample-data
Go to studio > one course and open the mange tags of any section/unit.
Check this changes: