Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

[CYS Woo Express] Large header #11390

Closed
Tracked by #11086
verofasulo opened this issue Oct 23, 2023 · 9 comments · Fixed by #11490
Closed
Tracked by #11086

[CYS Woo Express] Large header #11390

verofasulo opened this issue Oct 23, 2023 · 9 comments · Fixed by #11490
Assignees
Labels
priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue/PR concerns a confirmed bug.

Comments

@verofasulo
Copy link

Please review Large header.

How it currently looks like:

image image

How it's supposed to look like:

Figma - Large screens

image

Figma - Small screens

image

@verofasulo verofasulo added priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. ▫️ pattern: headers labels Oct 23, 2023
@nefeline
Copy link
Contributor

Quick note regarding the icon size: that will be worked on by Guidorah as part of woocommerce/woocommerce#40947 : we should not be hardcoding the width of the icon in patterns.

@nefeline nefeline added the type: bug The issue/PR concerns a confirmed bug. label Oct 23, 2023
@gigitux
Copy link
Contributor

gigitux commented Oct 24, 2023

Unfortunately, we can't achieve the mobile view. As discussed earlier, we can only "hide" the navigation block in the mobile view.

We can't:

  • change the position of the Navigation block (desktop view is at the center, on mobile view is on the right)
  • hide specific blocks (Customer Account block and Search Block)

It is worth mentioning that it is possible to hide the Search Block only if it is an inner block of the Navigation block.

cc @verofasulo @nefeline

@verofasulo
Copy link
Author

After discussing it on Slack, it looks like this is the only viable—and temporary—solution for mobile:

image

image

Let me know if you have any further questions 🙏

@gigitux gigitux self-assigned this Oct 25, 2023
@gigitux
Copy link
Contributor

gigitux commented Oct 26, 2023

I tried to implement this pattern, but the current approach doesn't work on mobile.

The first row is too large.

image

cc @verofasulo

@verofasulo
Copy link
Author

Thanks for trying it out, @gigitux! 🙏

The first row is too large.

I'm not sure I got the issue 😅

image

Can't we:

  • Align the search bar on the left side;
  • Apply the correct spacing between the account and cart icons and align them to the right;
  • Apply the correct spacing between the site title and the menu and center-align them?

@gigitux
Copy link
Contributor

gigitux commented Oct 30, 2023

As I already mentioned above, on very small screens, the Mini Cart icon falls outside of the screen. For example, this is what happens on the iPhone SE:

image

There are two fixes:

  • Allowing to wrap to multiple lines when there isn't enough space to show the icons on the same row:

image

  • get rid of the Product Search Block.

Any feedback, @verofasulo?

cc @albarin

@gigitux gigitux modified the milestone: 11.4.3 Oct 30, 2023
@verofasulo
Copy link
Author

As I already mentioned above, on very small screens, the Mini Cart icon falls outside of the screen.

What I was suggesting was to fix the spacing between the cart and the account icons, which is currently wrong, and see if it improves the mobile screen too. The correct spacing is 16px:

image

I'm also not getting why the site title and the hamburger menu are misaligned:

image

@gigitux
Copy link
Contributor

gigitux commented Oct 30, 2023

@verofasulo thanks for the feedback, but it is the same issue:

image

Even if I put 0px, on some devices, there is the issue:

image

@verofasulo
Copy link
Author

Adding a comment here for visibility: we decided to change the header design, both on large and narrow screens.

@gigitux proposed to replicate Zalando header, and it looks really good — here's an initial video and here's the convo on Slack.

@gigitux, could you please share a link to the header here once you're done so that I can have a last look? Thank you!

cc @albarin

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue/PR concerns a confirmed bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants