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

404: Offset "oops" to match design #323

Merged
merged 2 commits into from
Feb 17, 2022
Merged

404: Offset "oops" to match design #323

merged 2 commits into from
Feb 17, 2022

Conversation

iandunn
Copy link
Member

@iandunn iandunn commented Feb 16, 2022

Fixes #311

Follow up to #314.

  • Moves "oops" to be partially off screen.
  • removes an invalid margin-top that wasn't affecting anything
  • standardizes the local header height var
  • adds specificity to the search form styles, to prevent them bleeding into the global header search

The black footer logo is just something off in my environment (also on trunk). I don't think it'll affect staging/prod.

Screen Shot 2022-02-16 at 09 08 47-fullpage

Screen Shot 2022-02-16 at 09 08 57-fullpage

@iandunn iandunn requested review from ryelle and beafialho February 16, 2022 17:14
@ryelle
Copy link
Contributor

ryelle commented Feb 16, 2022

The footer logo isn't just your env, it's an issue from the logo change yesterday. See WordPress/wporg-mu-plugins#162

@iandunn iandunn merged commit ed776c7 into trunk Feb 17, 2022
@iandunn iandunn deleted the 404-oops-offset branch February 17, 2022 14:48
@beafialho
Copy link
Collaborator

Thank you, on mobile it looks good 👍

On desktop, it looks like there's too much space below the content, can we match that with the design? Also, the search field seems to be too high, let's make it 40px high, as shown on Figma.

Screenshot Mockups
Screenshot Mockup

@iandunn
Copy link
Member Author

iandunn commented Feb 17, 2022

I reduced the gap a bit in 74ca6fa (not deployed yet), but it's difficult to get it exact at every breakpoint, because of the dynamic font size.

I don't think most of the extra gap is coming from that, though. When the viewport is bigger than the page content, the footer sticks to the bottom, so the extra gap is expected to some degree.

iandunn added a commit that referenced this pull request Feb 17, 2022
@iandunn
Copy link
Member Author

iandunn commented Feb 17, 2022

a9fd18e fixes the search spacing. i'm going into a meeting, but will deploy it after that

@iandunn
Copy link
Member Author

iandunn commented Feb 17, 2022

Looks like @ryelle synced and deployed it, thanks!

@beafialho , you can see this on production now. LMK if you'd like any other changes.

@beafialho
Copy link
Collaborator

Thank you @iandunn, there's still too much space between the content and the footer, let's try to make the content sit in the middle of the page so that there's similar space above and below:

Captura de ecrã 2022-02-18, às 11 02 32

@iandunn
Copy link
Member Author

iandunn commented Feb 18, 2022

Ah, ok, I didn't realize you wanted it vertically centered. I'll look into that.

@iandunn
Copy link
Member Author

iandunn commented Feb 18, 2022

I got that mostly working in 388a4f0. It stops working once the viewport is 1050px tall, because of the sticky footer issue mentioned above. I couldn't find a good way around that. It still looks good at those sizes, though, just different.

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.

Implement 404 Page according to the design
3 participants