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

feat: enhanced undo ctrl panel #6545

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Conversation

giuliaghisini
Copy link
Contributor

@giuliaghisini giuliaghisini commented Dec 17, 2024

Enhanced Undo controlpanel.

Now it displays a loader while waiting for transactions from Plone.
When you want to delete some transaction, a confirm popup asks the user to confirm undo action for selected items.
Changed columns width to better display path.
Added select-all.
Added pagination.
Added OP column.
Added some translations.

Registrazione.schermo.2024-12-19.alle.12.43.49.mov

Enlarged the width of container in controlpanels, to have more horizontal space.
Unified the controlpanel's look to have same layout, some controlpanel had old layout, like

  • Content types
  • Moderate comments
  • Relations
    controlpanels.
undo.mov

Is it possibile to cherry-pick this PR in 17.x.x ?

Copy link

netlify bot commented Dec 17, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 4ab2c7d
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/677f8a17fef75a00088d2e65

@stevepiercy
Copy link
Collaborator

For accessibility, the issue of standardizing colors came up in:

The topic was added as an item to the PLIP.

@stevepiercy stevepiercy requested a review from a team December 18, 2024 12:20
@giuliaghisini
Copy link
Contributor Author

For accessibility, the issue of standardizing colors came up in:

The topic was added as an item to the PLIP.

ok, ill'remove this fixes

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Mostly comments on the .pot file to apply throughout the .po files, and expand the change log entry.

packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/locales/volto.pot Show resolved Hide resolved
packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/locales/volto.pot Outdated Show resolved Hide resolved
packages/volto/news/6545.feature Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Collaborator

ok, ill'remove this fixes

I think it's OK to leave them in, so we can see what you came up with and discuss it. I did not mean for you to delete them. Sorry for the miscommunication.

@giuliaghisini
Copy link
Contributor Author

ok, ill'remove this fixes

I think it's OK to leave them in, so we can see what you came up with and discuss it. I did not mean for you to delete them. Sorry for the miscommunication.

don't worry. I prefered to remove that styles because out of this pr. It's better to manage a11y styles in a single consistent pr.

@tisto
Copy link
Member

tisto commented Jan 2, 2025

@giuliaghisini amazing work! Thank you, Giulia!

This might be beyond the scope of this enhancement. However, I think we need to broaden the width of the control panels in Volto. The undo control panel, the redirection control panel, and other control panels that display lots of data in a table just do not have sufficient space. cc @sneridagh

@giuliaghisini
Copy link
Contributor Author

This might be beyond the scope of this enhancement. However, I think we need to broaden the width of the control panels in Volto. The undo control panel, the redirection control panel, and other control panels that display lots of data in a table just do not have sufficient space. cc

i agree... had some difficults to gain width for path column..
maybe... for all controlpanels.. we could think to enlarge 'contents' width to 80% of monitor width for example..

@giuliaghisini
Copy link
Contributor Author

@giuliaghisini amazing work! Thank you, Giulia!

This might be beyond the scope of this enhancement. However, I think we need to broaden the width of the control panels in Volto. The undo control panel, the redirection control panel, and other control panels that display lots of data in a table just do not have sufficient space. cc @sneridagh

@tisto
Copy link
Member

tisto commented Jan 3, 2025

@giuliaghisini @sneridagh I would increase the size of the main area to 90% at least if that's technically possible. The control panels are for power users and they need as much space as possible.

@giuliaghisini giuliaghisini reopened this Jan 3, 2025
@giuliaghisini
Copy link
Contributor Author

giuliaghisini commented Jan 3, 2025

@giuliaghisini @sneridagh I would increase the size of the main area to 90% at least if that's technically possible. The control panels are for power users and they need as much space as possible.

@tisto @sneridagh
i've updated this pr where i have increased the width of controlpanels.
I've updated the description and the example video of this PR if you want to check the result

@tisto
Copy link
Member

tisto commented Jan 3, 2025

@giuliaghisini looks great! Can wait to see this PR merged and released!

@stevepiercy
Copy link
Collaborator

Would it be possible to deselect the checkbox for all items when paginating?

I also think I'm missing something. I can't see actual changes to content. I think it would be useful to preview a diff of revisions like in Wordpress before undoing a transaction, but that might not be possible or outside the scope of this PR.

@giuliaghisini
Copy link
Contributor Author

Would it be possible to deselect the checkbox for all items when paginating?

I also think I'm missing something. I can't see actual changes to content. I think it would be useful to preview a diff of revisions like in Wordpress before undoing a transaction, but that might not be possible or outside the scope of this PR.

yes, i think this is outside the scope of this pr, maybe you could open an issue

Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

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

LGTM, since it's CMSUI, it's not breaking, we can move forward.

@giuliaghisini some checks need attention.

@stevepiercy
Copy link
Collaborator

Would it be possible to deselect the checkbox for all items when paginating?

@giuliaghisini
Copy link
Contributor Author

Would it be possible to deselect the checkbox for all items when paginating?

this is an intentional feature, sometimes you want to undo a lot of actions in one single transaction..

@giuliaghisini
Copy link
Contributor Author

@sneridagh i dont'know what have to do to fix this check, i have this problem on another pr too : https://github.com/plone/volto/actions/runs/12686287603/job/35358301758?pr=6545

@stevepiercy
Copy link
Collaborator

Would it be possible to deselect the checkbox for all items when paginating?

this is an intentional feature, sometimes you want to undo a lot of actions in one single transaction..

Let me try to explain it differently.

In the video, I see you click the top checkbox on Page 1 to select all items on the page. Then I see you go to the next page. I see you deselect the top checkbox, then reselect it, to select all items on the second page. I think that is one click too many and it looks a tiny bit weird to me.

Instead on the second page, I would like to see the top checkbox become deselected automatically. Then you can click it one time to select all items.

@davisagli
Copy link
Member

The UI with checkboxes is not ideal here. (I know it's the same that we always had in the ZMI; it was bad there too.) Usually it's not possible to undo an old transaction unless you're also undoing all the transactions that happened since then. So I think it would make more sense to use radio buttons, and undo all transactions between now and the one that the user selects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants