-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feat/light dark mode implementation #380
base: release/job-posting-flow
Are you sure you want to change the base?
Feat/light dark mode implementation #380
Conversation
…ng-bug-with-clear fix: search input is sticky now also clear btn is stick under input
…ofile-by-hunter feat: notification sent to discord chanel
…input style: add rule for hide unnecessary icon on country input
…bile-view Fix/missing search input for mobile view
…l-fields-all-forms feat: adding info about optional fields in all forms
anchor button now ensures https
move ensureProtocol util from ui-system to platform-app
RELEASE: Bugfixes & improvements
SocialItems component uses <a href> instead of <AnchorButton>, so a separate fix is needed here
ensureProtocol in SocialItems
release: linkedin url bugfix
… into feat/light-dark-mode-implementation
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.
The code looks good, I left a few tiny suggestions.
However, I noticed a few issues on the working page
-
contrast problems on the light theme (especially on the landing page)
-
on page "/profiles" (when you are unlogged) - there's one component without a light theme
-
One icon on the landing page is missing color (in both themes)
By the way, is it possible to add transition: background-color to every component?
//styleName: Header 2; | ||
font-size: 32px; | ||
font-weight: 400; | ||
line-height: 40px; |
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.
<FAQSection /> | ||
</Container> | ||
<LandingFooter /> | ||
<ThemeWrapper> |
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 the layout would be a better place for the Wrapper.
<h1 className={styles.title}>Good Dev Hunting</h1> | ||
</Link> | ||
<GitHubStarsButton /> | ||
<ThemeWrapper> |
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.
Using the theme wrapper in the page/layout will eliminate the need for it in this component
@@ -79,7 +79,7 @@ | |||
} | |||
|
|||
.name { | |||
color: #e2eaf1; | |||
color: var(--text-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.
Using CSS variables for colors improves maintainability and theme support. Good job!
b1b97eb
to
75496b1
Compare
No description provided.