Skip to content
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

Improving Feed link UI colours #2698

Merged
merged 5 commits into from
Apr 1, 2023
Merged

Conversation

rkmdCodes
Copy link
Contributor

I have resolved issue #2697.

Copy link
Contributor

@hughrun hughrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dark theme needs to also be updated, since it uses the same colour for links and the active tab.

@rkmdCodes
Copy link
Contributor Author

hi @hughrun sir thanks for reviewing my PR, currently i am unable to find the dark mode option in original site to test it in production , so can you please elaborate more about it? I have a little idea that i have to do same changes in theme-dark file too.

@hughrun
Copy link
Contributor

hughrun commented Mar 19, 2023

@rkmdCodes the dark mode sass file should be in the same directory as the light theme, in bookwyrm-dark.scss.

However, a better way to achieve this would be to edit bookwyrm/components/_tabs.scss by swapping $text and $link, which will then ensure it is correct in all themes:

.bw-tabs a {
    ...
    color: $link;
    ...
}

.bw-tabs a:hover {
    ...
    color: $link;
}

.bw-tabs a.is-active {
    ...
    color: $text;
}

@rkmdCodes
Copy link
Contributor Author

hi @hughrun sir , I have updated the _tabs.scss file and the issue is fixed. Thanks

@hughrun
Copy link
Contributor

hughrun commented Mar 27, 2023

@rkmdCodes hmm looks like my suggestion doesn't work - did you test this?

@rkmdCodes
Copy link
Contributor Author

hi @hughrun , I have resolved and reimplemented the theme colors for both light and dark theme and I've tested them too. Thanks for the feedback!
image
image

@rkmdCodes
Copy link
Contributor Author

hi @mouse-reeve can u please take a look at this PR ? Thank You

Copy link
Contributor

@hughrun hughrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I led you down the wrong path there! This works, thanks :)

@hughrun hughrun merged commit 499ff58 into bookwyrm-social:main Apr 1, 2023
@rkmdCodes
Copy link
Contributor Author

Thank You @hughrun for approving changes and merging my PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants