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

Prevent deletion of specific pages in site editor #67790

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

benazeer-ben
Copy link
Contributor

@benazeer-ben benazeer-ben commented Dec 10, 2024

What?

Fixes #64645

Why?

As per the issue mentioned in the ticket,
If you set a static homepage, then delete that page, the site will revert to displaying latest posts.

This can be a bit unexpected, especially if the site doesn't utilise a blog. It could improve the overall UX to place a guardrail here, preventing users from deleting the homepage.

Same scenario is applying to postage as well.

How?

Disabling 'Move to trash' option form front page and home page.

Testing Instructions

Go to WP admin dashboard
Go to Appearance -> Editor
Navigate to Pages
Try to delete page which is assigned to Home page or Post page.
'Move to Trash' option will be disabled fro both these type of pages.

Screenshots or screencast

Before After
movetrash aftermove

Copy link

github-actions bot commented Dec 10, 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: benazeer-ben <benazeer@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: fcoveram <fcoveram@git.wordpress.org>
Co-authored-by: oandregal <oandregal@git.wordpress.org>

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

@Mamaduka Mamaduka requested a review from youknowriad December 10, 2024 12:24
@Mamaduka Mamaduka added [Type] Enhancement A suggestion for improvement. [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") labels Dec 10, 2024
@benazeer-ben benazeer-ben changed the title Prevent deletion og specific pages in site editor Prevent deletion of specific pages in site editor Dec 19, 2024
@jameskoster
Copy link
Contributor

Thanks for the PR. I reckon it's okay for the 'Move to trash' action to remain, but in a disabled state. There's a design in #68134 that we could follow.

@benazeer-ben
Copy link
Contributor Author

Thanks for pointing this out, @jameskoster.

After facing some difficulties in implementing the exact design mentioned in #68134, I’ve made a slight adjustment to the approach. Instead of disabling the 'Move to Trash' button entirely, I’ve disabled the trash functionality and added an extra message within the modal when clicking 'Move to Trash'.

This message informs users that they need to select another page as the Front Page or Posts Page before deleting the current one if it is assigned as such.

Please share your feedback on this approach, and let me know if there’s anything else I should adjust or further refine.

Screenshare.-.2024-12-20.5_13_04.PM.mp4

@jameskoster
Copy link
Contributor

Hey, would you mind describing the difficulties you ran in to? It may be useful feedback for the folks working on the DataViews component. cc @ntsekouras @oandregal @jorgefilipecosta

I don't mind using a modal if we're unable to make the action disabled. In fact the additional context explaining why the page can't be deleted might be helpful. It can probably be a bit simpler though, something like this:

Screenshot 2024-12-20 at 14 37 24

Another consideration occurs to me; what should be done with the 'Move to trash' button in the Inspector, should it be consistent?

Screenshot 2024-12-20 at 14 29 12

cc @fcoveram as you've been exploring this with me :)

@fcoveram
Copy link
Contributor

thanks @benazeer-ben for sharing the exploration.

I think we need to prevent unexpected scenarios like landing on a screen that asks users to leave the flow where they are currently. In #68134 I shared two approaches proper to both Site and Block editor views:

  1. Disable the action ("Move to trash") and include a help text indicating the reason.
  2. Steer the flow by pulling in the replacement action.

If the first is doable, the action in Inspector can be disabled and include a text with a link to the page section in Site Editor. Something like the following.

If the first is undoable, I'm drawn to the second one. It's not alarming, it offers a solution. and seems to work well in the Inspector. Because of that, I'm starting to like the second path more.

@oandregal
Copy link
Member

oandregal commented Dec 24, 2024

Steer the flow by pulling in the replacement action.

I like this one. It teaches the user and enables them to perform the pre-requisite directly. It can even be a new action setNewHomepageBeforeDeleting that actually does the two things.

Hey, would you mind describing the difficulties you ran in to? It may be useful feedback for the folks working on the DataViews component.

Actions don't have a description — additionally, the design here makes the description dependent on the enabled/disabled state of the action, which is a bit more complex to implement.

I've also found that the disabled state of actions wasn't being respected in the UI: I'm fixing this at #68275

@benazeer-ben
Copy link
Contributor Author

I attempted using action.disabled for our scenario, which successfully disabled the "Move to Trash" action but did not allow us to specify conditions. Hence, I implemented the second solution suggested by @fcoveram. It appears to be working as expected and has been included in the recent commit.

Please review the changes and let me know if further modifications are required.

Screenshare.-.2025-01-08.5_09_34.PM.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent deletion of page_on_front
5 participants