Skip to content

Commit

Permalink
feat: improve project submisions dark mode (#4324)
Browse files Browse the repository at this point in the history
<!-- Thank you for taking the time to contribute to The Odin Project. In
order to get this pull request (PR) merged in a reasonable amount of
time, you must complete this entire template. -->

## Because
When visiting a full project submissions page the UX of dark mode was
worse of light mode potentially due to some rules not applying.

## This PR
- Gives the active page the lovely Odin gold colour just like light mode
does.
- Attempts to harmonize link colour with the `Back to lesson` link at
the top of the page.
- Makes disabled `Prev` / `Next` more visually distinct and intuitively
disabled depending on the circumstances.

## Issue

Closes #4307 

## Additional Information

This is directly pertaining to the PR, rest is about the process more
than anything else:
I duplicated the rules like `text-gold-500` because of specificity,
otherwise it wouldn't work. Looks weird, feels weird but works just
fine! Not sure if those `&:active` rules are even needed - it is only
seen for a split second on slow network after clicking a link, right?

According to the [Contributing
Guide](https://github.com/TheOdinProject/theodinproject/blob/main/CONTRIBUTING.md#top-website-repo)
you're supposed to run `rubocop` and not `bundle exec rubocop` - same
with `rspec`. Is this intentional? I have offences with `rubocop`, all
to do with `Capybara` but when I do `bundle exec rubocop` I don't see
any issues. Neither the main` rubocop.yml` nor the one in `spec/` have
any rules that seem to affect `Capybara`. It seems like my global
rubocop-capybara is `2.20.0` and the one in the Gemfile is `2.19.0` and
this is the reason for mismatch. I have already submitted a PR to remove
`yarn test` from the guide - it feels like every time a gem is used, it
should be used with `bundle exec`. Perhaps this should also be the case
with `rails` - every instance should be `bin/rails` instead.

Also, I have no idea if it is my setup or what, but there was plenty of
`bin/rails assets:clobber` and `bin/rails css:clobber` for me before I
could get webpack to actually reflect changes in the file on refresh. At
some point I was thinking the changes should not be made in `pagy.css`!

## Pull Request Requirements
<!-- Replace the whitespace between the square brackets with an 'x',
e.g. [x]. After you create the PR, they will become checkboxes that you
can click on. -->
- [x] I have thoroughly read and understand [The Odin Project
Contributing
Guide](https://github.com/TheOdinProject/theodinproject/blob/main/CONTRIBUTING.md)
- [x] The title of this PR follows the `keyword: brief description of
change` format, using one of the following keywords:
    - `Feature` - adds new or amends existing user-facing behavior
- `Chore` - changes that have no user-facing value, refactors,
dependency bumps, etc
    - `Fix` - bug fixes
-   [x] The `Because` section summarizes the reason for this PR
- [x] The `This PR` section has a bullet point list describing the
changes in this PR
- [x] I have verified all tests and linters pass after making these
changes.
- [x] If this PR addresses an open issue, it is linked in the `Issue`
section
-   [ ] If applicable, this PR includes new or updated automated tests
  • Loading branch information
scheals authored Jan 10, 2024
1 parent 5f7393b commit e5be408
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions app/assets/stylesheets/stylesheets/pagy.css
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
.pagy-nav-js .page.active,
.pagy-nav-js .page.prev.disabled,
.pagy-nav-js .page.next.disabled {
@apply block px-3 py-1 text-gray-500 dark:text-gray-300 font-semibold;
@apply block px-3 py-1 text-gray-500 dark:text-gray-400 font-semibold;
&:hover{
@apply text-gray-700 dark:text-gray-200;
}
Expand All @@ -23,23 +23,23 @@
.pagy-nav .page.next.disabled,
.pagy-nav-js .page.prev.disabled,
.pagy-nav-js .page.next.disabled {
@apply text-gray-400 dark:text-gray-500 first-letter:cursor-default;
@apply text-gray-400 dark:text-gray-700 first-letter:cursor-default;
&:hover {
@apply text-gray-400 dark:text-gray-500;
@apply text-gray-400 dark:text-gray-700;
}
&:active {
@apply text-gray-400 dark:text-gray-500;
@apply text-gray-400 dark:text-gray-700;
}
}

.pagy-nav .page.active,
.pagy-nav-js .page.active {
@apply cursor-default text-gold-500;
@apply cursor-default text-gold-500 dark:text-gold-500;
&:hover {
@apply text-gold-500;
@apply text-gold-500 dark:text-gold-500;
}
&:active {
@apply text-gold-500;
@apply text-gold-500 dark:text-gold-500;
}
}

Expand All @@ -51,10 +51,10 @@
.pagy-combo-nav-js .page.prev,
.pagy-combo-nav-js .page.next {
&:hover {
@apply text-gray-800;
@apply text-gray-800 dark:text-gray-200;
}
&:active {
@apply text-gray-800;
@apply text-gray-800 dark:text-gray-200;
}
}

Expand Down

0 comments on commit e5be408

Please sign in to comment.