-
Notifications
You must be signed in to change notification settings - Fork 36
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
💄 DOP-4654 applies dark mode support for product list component #1118
Conversation
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.
minor comments below! thanks for the catch with the landing heading!
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.
This LGTM 👍
@@ -62,7 +63,6 @@ const sideNavStyling = ({ hideMobile, isCollapsed }) => LeafyCSS` | |||
${isCollapsed && 'display: none;'} | |||
} | |||
|
|||
a, |
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 wondering what this change is
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.
@mmeigs this was applying the settings to the l > a
tags in the product list. I had to remove it to allow the dark mode styles to work within the View all products
.
src/templates/landing.js
Outdated
@@ -106,7 +106,7 @@ const Landing = ({ children, pageContext, useChatbot }) => { | |||
} | |||
} | |||
main h1:first-of-type { | |||
color: ${darkMode ? palette.gray.light2 : palette.black}; | |||
color: ${palette.gray.light2}; |
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.
this color is different from the header in production, which is #FFFFFF
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.
also causes a strange transition if you go to https://docs-mongodb-org-stg.s3.us-east-2.amazonaws.com/master/landing/caesarbell/DOP-4654/tools-and-connectors/
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.
@mayaraman19 when looking at tool-and-connectors in production it has the same green, or am I seeing it wrong https://www.mongodb.com/docs/tools-and-connectors/
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.
@mayaraman19 I will update the PR for to use the #fff, good catch on that
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.
@caesarbell to answer ur question, yes it uses the same green but as the font is changing to the green since i think those styles are being added, you can see the large grey font briefly.
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.
@caesarbell can you also update the staging after you've done so? :)
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 have updated the staging link. Are you not seeing the most recent?
src/templates/landing.js
Outdated
@@ -106,7 +106,7 @@ const Landing = ({ children, pageContext, useChatbot }) => { | |||
} | |||
} | |||
main h1:first-of-type { | |||
color: ${darkMode ? palette.gray.light2 : palette.black}; | |||
color: ${palette.gray.light2}; |
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.
Thanks to @mayaraman19 , I found this was causing a bug with the h1 on the homepage. I figure it was a small enough fix to include in this PR.
@@ -62,7 +63,6 @@ const sideNavStyling = ({ hideMobile, isCollapsed }) => LeafyCSS` | |||
${isCollapsed && 'display: none;'} | |||
} | |||
|
|||
a, |
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.
@mmeigs this was applying the settings to the l > a
tags in the product list. I had to remove it to allow the dark mode styles to work within the View all products
.
src/templates/landing.js
Outdated
@@ -106,7 +106,7 @@ const Landing = ({ children, pageContext, useChatbot }) => { | |||
} | |||
} | |||
main h1:first-of-type { | |||
color: ${darkMode ? palette.gray.light2 : palette.black}; | |||
color: ${palette.gray.light2}; |
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.
@mayaraman19 when looking at tool-and-connectors in production it has the same green, or am I seeing it wrong https://www.mongodb.com/docs/tools-and-connectors/
src/templates/landing.js
Outdated
@@ -106,7 +106,7 @@ const Landing = ({ children, pageContext, useChatbot }) => { | |||
} | |||
} | |||
main h1:first-of-type { | |||
color: ${darkMode ? palette.gray.light2 : palette.black}; | |||
color: ${palette.gray.light2}; |
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.
@mayaraman19 I will update the PR for to use the #fff, good catch on that
Stories/Links:
DOP-4654
Current Behavior:
Docs Landing
Staging Links:
Staged: Docs Landing
Notes:
This applies dark mode support to the Product List in the Side Nav.
README updates