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

DataViews: Allow column re-ordering. #63416

Merged
merged 1 commit into from
Jul 15, 2024
Merged

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Jul 11, 2024

Closes #63414

What?

This PR implements column reordering in table dataviews.

Notes:

  • This skips combined fields for now but I can implement them if needed, it's weird that we'll only have "move left" and "move right" in the dropdown of these fields though. That's why I chose to not show them there for now.
  • This highlights an actual bug elsewhere in the block editor iframe. If you move the "Preview" field to the right (you can do so by enabling it in the templates dataviews and moving it left and then right), you'll see a JS error in the console for the previews. Maybe @ellatrix could help here. It's a separate issue though and kind of unrelated with re-ordering.

Testing Instructions

  • Open any table dataviews (templates, patterns or pages)
  • Click the headers of the columns
  • Try "move left" and "move right".
@youknowriad youknowriad added [Type] Enhancement A suggestion for improvement. [Feature] Data Views Work surrounding upgrading and evolving views in the site editor and beyond labels Jul 11, 2024
@youknowriad youknowriad self-assigned this Jul 11, 2024
Copy link

github-actions bot commented Jul 11, 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: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>

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

</DropdownMenuGroup>
) }
<DropdownMenuGroup>
<DropdownMenuItem
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two additional menu items here are the main changes here, this component was just extracted from the "index.tsx" file otherwise.

@jameskoster
Copy link
Contributor

It's working for me:

Screenshot 2024-07-11 at 11 17 17
@ellatrix
Copy link
Member

This highlights an actual bug elsewhere in the block editor iframe. If you move the "Preview" field to the right (you can do so by enabling it in the templates dataviews and moving it left and then right), you'll see a JS error in the console for the previews. Maybe @ellatrix could help here. It's a separate issue though and kind of unrelated with re-ordering.

I'm not sure what steps I need to take to reproduce, could you file a separate issue if it's unrelated? I'm also not sure how to test this PR tbh, I don't see the move left/right buttons anywhere when I click the header of a column.

@youknowriad
Copy link
Contributor Author

@ellatrix yeah, sorry that was unclear. Here's a video that shows the bug.

Screen.Recording.2024-07-11.at.2.14.02.PM.mov
@jameskoster
Copy link
Contributor

A question for @WordPress/gutenberg-design (not necessary to address in this PR): should the menu remain open after moving rather than auto-closing?

@jasmussen
Copy link
Contributor

should the menu remain open after moving rather than auto-closing?

There are some separate discussion on that, e.g. this menu doesn't auto-close:

Screenshot 2024-07-11 at 14 54 30

I personally find that disruptive and confusing, and I think I'd find it confusing here too. But those are not strong opinions, rather to say: it may be a broader than just-design question.

@jameskoster
Copy link
Contributor

Probably something we should define at the system level 👍

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Code wise this looks good, thanks!

@youknowriad youknowriad merged commit 34e18e6 into trunk Jul 15, 2024
70 checks passed
@youknowriad youknowriad deleted the add/column-reordering branch July 15, 2024 14:46
@github-actions github-actions bot added this to the Gutenberg 18.9 milestone Jul 15, 2024
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jul 18, 2024
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Data Views Work surrounding upgrading and evolving views in the site editor and beyond [Type] Enhancement A suggestion for improvement.
5 participants