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

Navigation: The width of the responsive navigation is restricted to the wide width #44182

Open
iamtakashi opened this issue Sep 15, 2022 · 14 comments
Labels
[Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended

Comments

@iamtakashi
Copy link

Description

I've noticed the width of the responsive navigation has been restricted to the wide width. The change seems to be introduced in #43576, more specifically here.

This makes the icons jump if the theme uses the full width for the navigation.

This doesn't seem to be a theme-specific issue, but if I'm missing something obvious, kindly let me know. Thanks in advance!

Step-by-step reproduction instructions

  1. Use a theme that has a full-width header or create a variation of header.
  2. Add a navigation to the header and set the overlay to be 'always'.
  3. Make your browser wider than the wide width in the theme.
  4. Click the icon to open the overlay to see the position.

Screenshots, screen recording, code snippet

Currently:
Screen Recording 9-14-2022 at 11 01 PM

Without the plugin: This is the expected behaviour.
Screen Recording 9-14-2022 at 11 06 PM

Environment info

WP: v6.0.2
Gutenberg: trunk
Browser: Mac Chrome

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@artemiomorales artemiomorales added [Type] Bug An existing feature does not function as intended Needs Testing Needs further testing to be confirmed. [Block] Navigation Affects the Navigation Block labels Sep 15, 2022
@kathrynwp
Copy link

kathrynwp commented Sep 22, 2022

Hi @iamtakashi – thanks for the report. If I'm understanding this right, I think I can replicate this with Twenty Twenty-Two, but only if the Navigation block is set to right-aligned.

@jasmussen since it looks like you handled the PR Takashi referenced, would you be able to take a look, please? Thanks!

@iamtakashi
Copy link
Author

Hi @kathrynwp, thanks for your help.

The contents in the Twenty Twenty-Two header are restricted to the wide width as default, but if I make it full-width, l was able to see the issue regardless of the alignment. I don't believe many users make the header fill-width in Twenty Twenty-two :), but in this way, we can emulate the situation where the themes with a full-width header have.

@jasmussen
Copy link
Contributor

jasmussen commented Sep 26, 2022

Thank you for creating this issue! Yes indeed: right now the content inside the modal is restricted to the wide width, which happens to match TT3. This is a bandaid that makes it possible for themes to ensure the overlap of the two buttons, but the hard-coded limitation to just wide is one we need to remove, to be sure.

An interim solution here, it seems, would be if the navigation overlay could be aware of its parent template part width, and get its boundaries from that. A longer term ideal solution is to change the overlay menu to be a container block itself, so the contents can be fully customized (and edited in isolation).

CC: @Mamaduka just for a quick dev insight: how feasible does such an interim solution sound? Do you have any other thoughts here? Edit: This is the line of code that would be nice to inherit from a parent, somehow: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/navigation/style.scss#L668

@Mamaduka
Copy link
Member

Thanks for the ping, @jasmussen.

The inner blocks are usually wrapped in Group(s) for alignments. The TT2 is an excellent example of this. What happens if the Navigation block isn't a direct child of the template part? Which alignment should it inherit?

We can probably use the context to pass down the alignment, but as you mentioned, it would be a band-aid fix.

@getdave, @draganescu, any thoughts about this suggestion?

@jasmussen
Copy link
Contributor

Can "only ever inherit from the template part" be an answer?

@Mamaduka
Copy link
Member

Can "only ever inherit from the template part" be an answer?

I need to run some tests, but I think it can be done. I will post the update.

@jasmussen
Copy link
Contributor

Actually, that wouldn't make sense, since the TP would be the top level one. My bad. To turn it: can it instead be "only ever inherit from the immediate parent"? That seems like it should cover this issue 🤔

@draganescu
Copy link
Contributor

My opinion is the band aid is skippable :) and we should focus on figuring out how to edit the overlay.

@Mamaduka Mamaduka removed the Needs Testing Needs further testing to be confirmed. label Sep 27, 2022
@getdave
Copy link
Contributor

getdave commented Oct 4, 2022

We could make the overlay a separate block with its own inner blocks which can include the Nav block and other types of blocks.

@jasmussen
Copy link
Contributor

@getdave Sounds like it could work, it's also been mentioned as a potential path forward for #43852. Should we rejigger that issue to become about a new overlay block? (And it would be sweet to edit this overlay block in focus mode when pressing the burger button!)

@getdave
Copy link
Contributor

getdave commented Oct 5, 2022

My main question is how do you configure the overlay block and the Nav block to show/hide depending on viewport size? I guess for now using some rudimentary "Mobile" toggle like we do for the Overlay setting in the Navigation block?

@jasmussen
Copy link
Contributor

Just to be sure I'm understanding you right, the overlay show/hide is already working, using this feature, no?

Screenshot 2022-10-06 at 09 08 58

@getdave
Copy link
Contributor

getdave commented Oct 6, 2022

Ah I was thinking of completely decoupling the overlay from the Navigation menu. This would allow you to have a different menu with different styling on "mobile". Unfortunately if the Overlay block is not coupled to the Nav block then the Overlay block will need to have the same Off | Mobile | Always type controls as you show in your screenshot above.

@jasmussen
Copy link
Contributor

Good questions to work through. I commandeered #43852 to be about the overlay as a block, it could be a good place to center the conversation and challenges, and I'll be here to update mockups for any shortcomings present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

7 participants