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

Updated UI for the local results page + support for dark mode #11

Merged
merged 2 commits into from
Dec 13, 2021
Merged

Updated UI for the local results page + support for dark mode #11

merged 2 commits into from
Dec 13, 2021

Conversation

nckrtl
Copy link
Contributor

@nckrtl nckrtl commented Dec 12, 2021

The changes for this pull request have been presented earlier in #10. The biggest changes are:

1. Updated results page
The results page now look like this:
list-web
The tiles have a minimum height to make them look a bit more uniform. Especially when there are failed checks, as the reason/explanation can cover 2 lines. When all checks are ok the extra height perhaps makes less sense. I don't find it bothering but if this is disliked I could restore the current behaviour (make tiles equal height on a row per row basis).

2. Dark mode
Support has been added for a new config key theme which can be set to dark to enable dark mode, the default is light:
list-web-dark

3. Blade components
2 blade component have been introduced, a logo component and a status indicator component. The main goal was to keep the list.blade.php compact as I used inline SVG's for the package logo/icon and the icons which emphasise the check status.

4. Tailwind v3
I added support Tailwind v3, which replaces the use of the CDN for Tailwind v2. For the sake of simplicity I decided to set it up with Tailwind CLI, which supports file scanning and minifying like PostCSS and cssnano. The implementation of including the css file has been inspired on how its done at https://github.com/spatie/laravel-dashboard (as suggested). I didn't copy all asset functions as I thought the inline stylesheet functionality would be enough for now.

5. Updated docs
In the docs the example config has been updated and the screenshot in the webpage section is also updated. Another screenshot has been added to highlight the dark mode option, accompanied with a guiding text on how to enable it.

Feedback
If there is anything that should be changed or reverted I'm more than happy to do so.

Kind regards,

Nick

Comment on lines +4 to +5
"watch": "npx tailwindcss -i ./resources/css/health.css -o ./resources/dist/health.min.css --watch",
"build": "npx tailwindcss -i ./resources/css/health.css -o ./resources/dist/health.min.css --minify"
Copy link
Contributor

Choose a reason for hiding this comment

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

We have tailwindcss listed as a dependency already, so I think we can just call that directly without having to rely on npx which might also pull a different version than the one we have specified in our package.json.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @abenerd as far as I know this should be isolated from your package.json in your project folder and would therefore not interfere with it.

@freekmurze freekmurze merged commit c177ce4 into spatie:main Dec 13, 2021
@freekmurze
Copy link
Member

Thank you very much for thins @nckrtl

This is seriously one of the best PRs I've ever seen 👍

@freekmurze
Copy link
Member

@nckrtl I think it would also be nice to follow os preference when choosing between light and dark mode.

If you want, you could send a PR with an new auto theme and use that as the default.

@nckrtl
Copy link
Contributor Author

nckrtl commented Dec 13, 2021

Thanks @freekmurze! I tried to add support for OS preference, but I didn't manage to get it to work as I had some trouble figuring out how to add assets in the blade view. Now I managed to get inline styling included I don't think it will be that hard to extend it with OS preference support. Will look into it when I have some time :)

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