-
Notifications
You must be signed in to change notification settings - Fork 23
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
Refine 404 styles #314
Refine 404 styles #314
Conversation
572b392
to
6d44fc2
Compare
6d44fc2
to
237b7df
Compare
Thanks @iandunn, I can't see it properly because my testing env is somehow stealing some space below the header, but besides the "Oops" element, the screenshots looks accurate 👍 |
I think you're behind on a Gutenberg update, or you've got it deactivated locally. |
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.
Overall this looks good, just a few comments, mostly accessibility-adjacent.
The "homepage" link here should be using var(--wp--preset--color--blue-3);
, and the search input could use a focus state (I don't know if there's one designed, @beafialho).
source/wp-content/themes/wporg-news-2021/block-templates/404.html
Outdated
Show resolved
Hide resolved
Ah, you're right, thanks! I had Git and SVN checkouts of it for various reasons, but accidentally updated it w/ wp-cli, which left the folder empty except for |
I think the only remaining thing is possibly the focus state. I'm going to merge this to make it easier to start on the follow-up PRs, and the focus state can be done in one of those based on Beatriz's feedback. |
See #311
Doesn't try to fix these, since they'll be easier to focus on in separate PRs, after this is merged.
home-link
block empty output. seems like a recent regression, it's happening on trunk, and the cause isn't obvious. maybe related to recent gutenberg release.