-
Notifications
You must be signed in to change notification settings - Fork 44
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
TreeView folder color #358
Conversation
mperrotti
commented
Sep 29, 2022
🦋 Changeset detectedLatest commit: f9124dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Variables changed--- base/dist/scss/colors/_dark.scss 2022-10-12 16:54:52.065385362 +0000
+++ dist/scss/colors/_dark.scss 2022-10-12 16:54:30.637588550 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: rgba(177,186,196,0.12);
+ --color-tree-view-item-directory-fill: #8b949e;
--color-fg-default: #c9d1d9;
--- base/dist/scss/colors/_dark_colorblind.scss 2022-10-12 16:54:52.077385222 +0000
+++ dist/scss/colors/_dark_colorblind.scss 2022-10-12 16:54:30.653588360 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: rgba(177,186,196,0.12);
+ --color-tree-view-item-directory-fill: #8b949e;
--color-fg-default: #c9d1d9;
--- base/dist/scss/colors/_dark_dimmed.scss 2022-10-12 16:54:52.069385315 +0000
+++ dist/scss/colors/_dark_dimmed.scss 2022-10-12 16:54:30.641588502 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: rgba(144,157,171,0.12);
+ --color-tree-view-item-directory-fill: #768390;
--color-fg-default: #adbac7;
--- base/dist/scss/colors/_dark_high_contrast.scss 2022-10-12 16:54:52.073385268 +0000
+++ dist/scss/colors/_dark_high_contrast.scss 2022-10-12 16:54:30.645588455 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: #525964;
+ --color-tree-view-item-directory-fill: #f0f3f6;
--color-fg-default: #f0f3f6;
--- base/dist/scss/colors/_dark_tritanopia.scss 2022-10-12 16:54:52.081385175 +0000
+++ dist/scss/colors/_dark_tritanopia.scss 2022-10-12 16:54:30.653588360 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: rgba(177,186,196,0.12);
+ --color-tree-view-item-directory-fill: #8b949e;
--color-fg-default: #c9d1d9;
--- base/dist/scss/colors/_light.scss 2022-10-12 16:54:52.029385784 +0000
+++ dist/scss/colors/_light.scss 2022-10-12 16:54:30.601588975 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: rgba(208,215,222,0.32);
+ --color-tree-view-item-directory-fill: #54aeff;
--color-fg-default: #24292f;
--- base/dist/scss/colors/_light_colorblind.scss 2022-10-12 16:54:52.049385549 +0000
+++ dist/scss/colors/_light_colorblind.scss 2022-10-12 16:54:30.617588786 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: rgba(208,215,222,0.32);
+ --color-tree-view-item-directory-fill: #54aeff;
--color-fg-default: #24292f;
--- base/dist/scss/colors/_light_high_contrast.scss 2022-10-12 16:54:52.045385596 +0000
+++ dist/scss/colors/_light_high_contrast.scss 2022-10-12 16:54:30.613588833 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: #ced5dc;
+ --color-tree-view-item-directory-fill: #368cf9;
--color-fg-default: #0e1116;
--- base/dist/scss/colors/_light_tritanopia.scss 2022-10-12 16:54:52.061385409 +0000
+++ dist/scss/colors/_light_tritanopia.scss 2022-10-12 16:54:30.625588691 +0000
@@ -285,2 +285,3 @@
--color-tree-view-item-chevron-hover-bg: rgba(208,215,222,0.32);
+ --color-tree-view-item-directory-fill: #54aeff;
--color-fg-default: #24292f; |
🟢 No design token changes found |
data/colors/vars/component_dark.ts
Outdated
@@ -209,6 +209,9 @@ export default { | |||
treeViewItem: { | |||
chevron: { | |||
hoverBg: alpha(get('scale.gray.2'), 0.12), | |||
}, | |||
folder: { | |||
fill: '#4579CA' |
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.
Is there a scale value this can reference?
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 currently use fg-muted
for dark mode directory icons in dotcom: https://cs.github.com/github/github/blob/e7f27d4cf8902dc3c9e6d8d908f066db54ab4020/app/assets/stylesheets/bundles/global/hx_color-modes.scss?q=org%3Agithub+--color-icon-directory#L34
fill: '#4579CA' | |
fill: get('fg.muted') |
folder: { | ||
fill: get('accent.muted') |
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've been calling it directory
instead of folder
to align with the octicon naming: https://primer.style/octicons/file-directory-fill-16
folder: { | |
fill: get('accent.muted') | |
directory: { | |
fill: get('accent.muted') |
data/colors/vars/component_light.ts
Outdated
@@ -210,6 +210,9 @@ export default { | |||
treeViewItem: { | |||
chevron: { | |||
hoverBg: alpha(get('scale.gray.2'), 0.32), | |||
}, | |||
folder: { | |||
fill: '#79B8FF' |
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 currently use scale-blue-3
for light mode directory icons in dotcom: https://cs.github.com/github/github/blob/e7f27d4cf8902dc3c9e6d8d908f066db54ab4020/app/assets/stylesheets/bundles/global/hx_color-modes.scss?q=org%3Agithub+--color-icon-directory#L34
fill: '#79B8FF' | |
fill: get('scale.blue.3') |
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.
@colebemis I've been calling it directory instead of folder to align with the octicon naming: https://primer.style/octicons/file-directory-fill-16
@mperrotti Do you have a strong preference for folder
? Otherwise, it seems reasonable to align the naming with Octicons (directory
).
I agree with @colebemis that it should be |