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

Refactor "Display" panel of Navigation block to use ToolsPanel instead of PanelBody #67935

Open
fabiankaegy opened this issue Dec 13, 2024 · 4 comments · May be fixed by #68011
Open

Refactor "Display" panel of Navigation block to use ToolsPanel instead of PanelBody #67935

fabiankaegy opened this issue Dec 13, 2024 · 4 comments · May be fixed by #68011
Labels
[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.

Comments

@fabiankaegy
Copy link
Member

fabiankaegy commented Dec 13, 2024

Part of #67813

Convert the "Settings" panel of the Navigation block to use the ToolsPanel instead of PanelBody

@yogeshbhutkar
Copy link
Contributor

I'd like to work on this issue.

Also, there's some discrepancy between the issue title and description. I think the intention was to mention the Display Panel originally, so I would go with refactoring the Display Panel.

Thanks!

@yogeshbhutkar
Copy link
Contributor

yogeshbhutkar commented Dec 16, 2024

A pull request addressing this issue has recently been raised: #68011. I will discontinue my work on this in favor of the existing PR.

@fabiankaegy fabiankaegy linked a pull request Dec 16, 2024 that will close this issue
@SainathPoojary
Copy link
Contributor

Hey @fabiankaegy,

The Navigation block contains multiple non-control items, which is causing layout issues. Additionally, some items have margin-bottom and extra styling applied to improve the layout in PanelBody, but this looks problematic in ToolsPanel.

I would like to clarify the best approach before proceeding. Should I write custom CSS to resolve the styling issue? If so, should I create custom CSS classes and apply them here instead? Or should I follow the approach used in Storybook and documentation, which utilizes styled components?

It’s worth noting that styled components are used in packages/components, but not in packages/block-library. If we go with styled components, we’ll need to add it to package.json.

Image

cc: @t-hamano @aaronrobertshaw

@aaronrobertshaw
Copy link
Contributor

The Navigation block contains multiple non-control items, which is causing layout issues.

The screenshot appears to be the expected behaviour as the ToolsPanel defaulted to a two-column grid layout. The ToolsPanelItem defaults to spanning both columns so removing that as a wrapper will mean needing to style grid-column for non-tools-panel-items.

You can see this in the example snippet I left in this comment on a related issue.

Should I write custom CSS to resolve the styling issue?

This is the approach that has been taken for the panels used by block supports e.g. color, dimensions, typography etc.

Ultimately, it might be nice for the ToolsPanel to offer different base layouts. I have a vague memory at one point this was implemented by eventually rolled back before the panel merged.

For the time being though, I'd suggest staying consistent in approach with the other block support panels.

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] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants