Merged
Conversation
c18374d to
e1a1450
Compare
timacdonald
approved these changes
Aug 8, 2022
Member
Author
|
Jetstream is also missing the dark variant on the nav links, so if this is merged, I'll make the same change over there. |
driesvints
approved these changes
Aug 8, 2022
slimani-dev
pushed a commit
to slimani-dev/breeze
that referenced
this pull request
Jan 21, 2023
* Remove unnecessary styles from Vue welcome page * Add missing dark variant for welcome nav links * Fix underlined trailing white space in Vue welcome page * Make StyleCI happy
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR primarily addresses the contrast and readability of the "Log in" and "Register" links in the Vue and React versions while using dark mode. I watched someone use Laravel for the first time, with the Breeze React stack, and they didn't notice the links.
It adds the missing
darktext color variant that is present in thewelcome.blade.phptemplate from the skeleton, which makes the text lighter when on a dark background.While making this change, I found that the
Welcome.vuepage had scoped styles that did not include the CSS class required for the above change. Rather than manually add the class, I chose to remove the scoped styles completely, in favour of using the classes provided by Tailwind. This is consistent with theWelcome.jsxpage from the React version.It's important to note that
welcome.blade.phprequires these styles to be included in the view because the skeleton does not come with Tailwind. It also ensures that users get a styled welcome page even when they haven't compiled their assets. The Vue and React versions always require a build step, and Breeze provides Tailwind, so the CSS classes don't need to be baked in.The only caveat is that the classes in
welcome.blade.phpuse a different color palette than Tailwind 3. The React version was already using the Tailwind colors, but now the Vue version is as well. If we want them to be consistent, I would suggest we change the baked-in classes inwelcome.blade.phpto use Tailwind 3 colors. If we prefer the warmer grays from the current skeleton, we could look at switching everything to one of the other grays that Tailwind 3 provides, such as Zinc or Neutral. Alternatively, we could add the custom palette to thetailwind.config.jsfile in this project.Additionally, the "Log in" link of the Vue version contained underlined white space after the link, so I've addressed this by removing the white space, which makes it consistent with
welcome.blade.php.Here are screenshots of the three stacks before any changes were made:
Blade before:
Vue before:
(Note the underlined whitespace after the "Log in" link)
React before:
The contrast on the React version is the worst, and it's not helped on a high-resolution screen where the links are very small and far away from the rest of the content on the page.
This PR doesn't change the Blade stack, so here is what the Vue and React versions look like after these changes:
Vue after:
React after: