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(content-distribution): Block the editor for incoming posts #181

Merged
merged 13 commits into from
Jan 20, 2025

Conversation

naxoc
Copy link
Member

@naxoc naxoc commented Jan 3, 2025

This adds the UI editor "blocking" for incoming posts.

The editor is disabled and save is not possible (from the UI). I haven't put any protections in place in PHP on save by API calls or similar. I can add that if we think it's necessary.

Note that I changed the text from "This post is linked to the origin post. Edits to the origin post will update this remote version" to "This post is linked to the origin post. Edits to the origin post will update this version" in the sidebar for linked posts.

I added the function get_original_site_url() – I stole it from #177 :)

@naxoc naxoc requested a review from miguelpeixe January 3, 2025 16:54
@naxoc naxoc self-assigned this Jan 3, 2025
@naxoc naxoc requested a review from a team as a code owner January 3, 2025 16:54
Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

Good solution blocking the block editor!

When editing a linked post, I'm still able to use the side panel forms:

image

These should either render disabled inputs or nothing at all. Not sure which route, WDYT?

When a distributed post comes in, it's expectedly a draft. When trying to publish, the button is disabled, possibly because of the lockPostSaving(). We still need to allow a post to be published (or any status change), can we find a way around that?

image

When a post gets unlinked, I'm still getting the editor blocked. I should be allowed to make changes when unlinking a post.

I'm also missing the ability to unlink/link. Will it be tackled separately? It might be convenient to handle in the same PR, since that change has a direct impact in how this UI should behave.

Comment on lines 71 to 75
'This post is linked to the origin post. Edits to the origin post will update this version',
'newspack-network'
)
: __(
'This post has been unlinked from the origin post. Edits to the origin post will not update this version',
Copy link
Member

Choose a reason for hiding this comment

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

NIT:

Suggested change
'This post is linked to the origin post. Edits to the origin post will update this version',
'newspack-network'
)
: __(
'This post has been unlinked from the origin post. Edits to the origin post will not update this version',
'This post is linked to the origin post. Edits to the origin post will update this version.',
'newspack-network'
)
: __(
'This post has been unlinked from the origin post. Edits to the origin post will not update this version.',

Comment on lines 27 to 30
useEffect(() => {
lockPostSaving('distributed-incoming-post-lock');
lockPostAutosaving('distributed-incoming-post-lock');
});
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be conditioned to isLinked

@naxoc
Copy link
Member Author

naxoc commented Jan 8, 2025

Thanks for the review!

I realized I am missing quite a few things here, so I'm back at the drawing board :)

I'll add/fix the things you are pointing out

@naxoc
Copy link
Member Author

naxoc commented Jan 9, 2025

OK, here is take two.

The CSS is still all that enforces the "blocking". Parts of it became a bit more fragile than I like, so I'm open to suggestions.

I added a component distribute-panel that we could re-use for the distribute panel for what is in the JS distribute component now (and maybe rename that to outgoing-post), but we can do that in a separate PR also.

How to test:

  • Distribute a post and play around with the UI.
  • Toggle the post linked and unlinked. A linked post should lock down:
    • The editor
    • The sidebar (execpt for the one that has the status, trash button, etc).
    • The pre-publish sidebar – except the bits we allow (please review that CSS and see if that is too fragile)
    • The bottom part of the editor where Yoast lives for instance.

@naxoc naxoc requested a review from miguelpeixe January 9, 2025 18:54
Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

Thank you for the revisions! It's looking great!

I added a component distribute-panel that we could re-use for the distribute panel for what is in the JS distribute component now (and maybe rename that to outgoing-post), but we can do that in a separate PR also.

I loved that idea! Let's do this after this gets merged.

I left a couple of comments below, but also wanted your thoughts on the relinking action. When a post is unlinked, you're able to make changes to it and relinking will overwrite those changes. On the backend this is working well, but on the editor you'll only see the resynced content after a page refresh, not right after relinking is complete.

Ideally, we'd have that content resync instantly, but I'm not familiar with a way to make Gutenberg pull content and refresh its store from the backend. Do you have ideas?

src/content-distribution/incoming-post/index.js Outdated Show resolved Hide resolved
src/content-distribution/incoming-post/index.js Outdated Show resolved Hide resolved
@naxoc
Copy link
Member Author

naxoc commented Jan 15, 2025

I re-did things a bit and spent a bunch of time researching whether it's possible to replace all content in the editor on relinking. The post content and title is easy enough, but I could find no way to guarantee that I would not overwrite the tags, categories, or whatever else was edited, so I went with just reloading the page on relinking. That created its own problems to avoid the browser dialogue with "You have unsaved changes" so I have quite a lot of async chaining in the code to work around that. Suggestions welcome to make that more sleek!

I also renamed the panel component and fixed up the CSS a bit more.

@naxoc naxoc requested a review from miguelpeixe January 15, 2025 19:20
Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

Thank you for the revisions, @naxoc! It's working well and the UI is solid.

I found 2 things that we can tackle in another PR:

  • When the post is linked (UI blocked), if I navigate to the post settings panel and click on the "Block" tab, I can't click back to the "Post" tab. Something about the UI blocking strategy takes over that area as well.
  • An oversight on my part, we should also block the ability to change the featured image:
Captura de Tela 2025-01-16 às 09 38 38

@naxoc naxoc merged commit 48b4cae into trunk Jan 20, 2025
3 of 4 checks passed
Copy link

Hey @naxoc, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

@naxoc naxoc deleted the feat/content-distribution-editor-block branch January 20, 2025 10:02
naxoc added a commit that referenced this pull request Jan 20, 2025
This fixes 2 things that came out of #181

* Also blocks the "Featured image" button so that is not editable on
linked posts
* Fixes a bug where it was not possible to click from the "Block" tab in
the sidebar to the "Post" tab.

## How to test
On a linked post – ensure that the sidebar blocks what it should (and no
more) and that you can click between the tabs in the sidebar.
naxoc added a commit that referenced this pull request Jan 20, 2025
This fixes 2 things that came out of #181

* Also blocks the "Featured image" button so that is not editable on
linked posts
* Fixes a bug where it was not possible to click from the "Block" tab in
the sidebar to the "Post" tab.

## How to test
On a linked post – ensure that the sidebar blocks what it should (and no
more) and that you can click between the tabs in the sidebar.
matticbot pushed a commit that referenced this pull request Jan 23, 2025
# [2.5.0-alpha.1](v2.4.0...v2.5.0-alpha.1) (2025-01-23)

### Bug Fixes

* **content-distribution:** block additional UI components ([#197](#197)) ([3f30cdb](3f30cdb))
* **content-distribution:** handling multiple post meta ([#199](#199)) ([c93084d](c93084d))
* **content-distribution:** Improve CSS for blocked editor ([#193](#193)) ([295bdad](295bdad)), closes [#181](#181)
* **content-distribution:** persist site hash ([#186](#186)) ([120b759](120b759))
* **content-distribution:** prevent consecutive dispatches ([#198](#198)) ([2149a62](2149a62))
* **content-distribution:** refactor outgoing post js ([#188](#188)) ([cc08edc](cc08edc))
* **memberships:** remove managed fields on cancel or expire ([#192](#192)) ([67a5cf0](67a5cf0))

### Features

* **content-distribution:** Block the editor for incoming posts ([#181](#181)) ([48b4cae](48b4cae))
* **content-distribution:** confirm dialog for unlinking and relinking posts ([#190](#190)) ([f36bb28](f36bb28))
* **content-distribution:** migrator ([#185](#185)) ([06ec18a](06ec18a))
* **content-distribution:** post status on create ([#189](#189)) ([1ad3e0c](1ad3e0c))
* sync multiple user roles ([#187](#187)) ([9fa833c](9fa833c))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.5.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants