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

Menu granular subcomponents: Refactor dataviews table layout header menu #67640

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Dec 5, 2024

What?

In the context of #67422, refactor the Menu component used in the header of the table layout in the dataviews package

Why?

See #67422

How?

Refactor usages of the Menu component. See #67422 for more details on how to refactor from the old to the new APIs.

Testing Instructions

  • Open the "Pages" view in the site editor with the "table" layout by visiting the link /wp-admin/site-editor.php?p=%2Fpage&layout=table (visiting the link is necessary because the layout switcher dropdown menu is being refactored in a separate PR and therefore won't work in this PR)
  • Make sure that the dropdown menus in the table header look and work like on trunk

Note

Note that test failures and potential build/runtime errors are expected while the various PR extracted from #67422 (as the current one) are not merged back into #67422 yet.

Screenshot 2024-12-05 at 16 56 43

@ciampo ciampo mentioned this pull request Dec 5, 2024
3 tasks
@ciampo ciampo self-assigned this Dec 5, 2024
@ciampo ciampo added [Type] Task Issues or PRs that have been broken down into an individual action to take [Package] DataViews /packages/dataviews labels Dec 5, 2024
@ciampo ciampo marked this pull request as ready for review December 5, 2024 21:29
Copy link

github-actions bot commented Dec 5, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: oandregal <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests well and code LGTM 👍

</Button>
}
style={ { minWidth: '240px' } }
// align="start" TODO: align prop doesn't exist on Menu — is it ok to just remove
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine, I don't see a difference by removing it. I don't see such a prop in Ariakit's Menu API either. Maybe this was a bad copy/pasta?

@oandregal
Copy link
Member

Tested the menu actions still work as expected. Code changes look good.

As with the other PR, I see some weird UI things, but it affects other Menu (not yet refactored):

Screenshot 2024-12-09 at 17 38 37

@tyxla
Copy link
Member

tyxla commented Dec 9, 2024

Tested the menu actions still work as expected. Code changes look good.

As with the other PR, I see some weird UI things, but it affects other Menu (not yet refactored):

@oandregal: @ciampo is splitting the changes into multiple PRs, so the idea is to review the affected parts per-piece only, ignoring the oddities outside of the PR's focus. So the oddities you're seeing are expected. Once merged into #67422, we'll review and test thoroughly again, and there should be no oddities.

@ciampo ciampo merged commit 37c9e53 into feat/menu-granular-trigger-components Dec 12, 2024
33 of 39 checks passed
@ciampo ciampo deleted the feat/menu-granular-components/dataviews-table-header branch December 12, 2024 09:31
ciampo added a commit that referenced this pull request Dec 12, 2024
ciampo added a commit that referenced this pull request Dec 13, 2024
ciampo added a commit that referenced this pull request Dec 16, 2024
ciampo added a commit that referenced this pull request Dec 16, 2024
* MenuItem: add render and store props

* Extract sub-components: popover, trigger button, submenu trigger item

* Unit tests

* CHANGELOG

* Add more memory to node on CI

* Refactor block bindings panel menu (#67633)

Co-authored-by: ciampo <[email protected]>

* Storybook (#67632)

* Refactor dataviews item actions menu (#67636)

* Refactor dataviews view config menu (#67637)

* Refactor global styles shadows edit panel menu (#67641)

* Refactor global styles font size menus (#67642)

* Refactor "Add filter" dataviews menu (#67634)

* Menu granular subcomponents: Refactor dataviews list layout actions menu (#67639)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: oandregal <[email protected]>

* Menu granular subcomponents: Refactor dataviews table layout header menu (#67640)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: oandregal <[email protected]>

* Menu granular subcomponents: Refactor post actions menu (#67645)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>

* Better comments for submenu trigger store

* Typo

* Remove unnecessary MenuSubmenuTriggerItemProps type

* Don't break the rules of hooks 🪝

* Move CHANGELOG entry to unreleased section

* Add explicit MenuProps to improve TS performance

* Remove node memory settings

---------

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: oandregal <[email protected]>
yogeshbhutkar pushed a commit to yogeshbhutkar/gutenberg that referenced this pull request Dec 18, 2024
* MenuItem: add render and store props

* Extract sub-components: popover, trigger button, submenu trigger item

* Unit tests

* CHANGELOG

* Add more memory to node on CI

* Refactor block bindings panel menu (WordPress#67633)

Co-authored-by: ciampo <[email protected]>

* Storybook (WordPress#67632)

* Refactor dataviews item actions menu (WordPress#67636)

* Refactor dataviews view config menu (WordPress#67637)

* Refactor global styles shadows edit panel menu (WordPress#67641)

* Refactor global styles font size menus (WordPress#67642)

* Refactor "Add filter" dataviews menu (WordPress#67634)

* Menu granular subcomponents: Refactor dataviews list layout actions menu (WordPress#67639)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: oandregal <[email protected]>

* Menu granular subcomponents: Refactor dataviews table layout header menu (WordPress#67640)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: oandregal <[email protected]>

* Menu granular subcomponents: Refactor post actions menu (WordPress#67645)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>

* Better comments for submenu trigger store

* Typo

* Remove unnecessary MenuSubmenuTriggerItemProps type

* Don't break the rules of hooks 🪝

* Move CHANGELOG entry to unreleased section

* Add explicit MenuProps to improve TS performance

* Remove node memory settings

---------

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: oandregal <[email protected]>
yogeshbhutkar pushed a commit to yogeshbhutkar/gutenberg that referenced this pull request Dec 18, 2024
* MenuItem: add render and store props

* Extract sub-components: popover, trigger button, submenu trigger item

* Unit tests

* CHANGELOG

* Add more memory to node on CI

* Refactor block bindings panel menu (WordPress#67633)

Co-authored-by: ciampo <[email protected]>

* Storybook (WordPress#67632)

* Refactor dataviews item actions menu (WordPress#67636)

* Refactor dataviews view config menu (WordPress#67637)

* Refactor global styles shadows edit panel menu (WordPress#67641)

* Refactor global styles font size menus (WordPress#67642)

* Refactor "Add filter" dataviews menu (WordPress#67634)

* Menu granular subcomponents: Refactor dataviews list layout actions menu (WordPress#67639)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: oandregal <[email protected]>

* Menu granular subcomponents: Refactor dataviews table layout header menu (WordPress#67640)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: oandregal <[email protected]>

* Menu granular subcomponents: Refactor post actions menu (WordPress#67645)

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>

* Better comments for submenu trigger store

* Typo

* Remove unnecessary MenuSubmenuTriggerItemProps type

* Don't break the rules of hooks 🪝

* Move CHANGELOG entry to unreleased section

* Add explicit MenuProps to improve TS performance

* Remove node memory settings

---------

Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: oandregal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] DataViews /packages/dataviews [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants