-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Export post permalink component #15943
Conversation
Sync Gutenberg
Sync form
@@ -33,6 +33,7 @@ export { default as PostLastRevisionCheck } from './post-last-revision/check'; | |||
export { default as PostLockedModal } from './post-locked-modal'; | |||
export { default as PostPendingStatus } from './post-pending-status'; | |||
export { default as PostPendingStatusCheck } from './post-pending-status/check'; | |||
export { default as PostPermalink } from './post-permalink'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution @igmoweb 👍 It seems fine to expose this component. Would it be possible to add a changelog entry specifying that this component is now exposed (new features section)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jorgefilipecosta Added. I hope it's fine this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
@youknowriad - can you perform sanity check before we merge? |
Now, that I think about it. We discussed removing that component entirely at some point since it's possible to edit the permalink in the sidebar. Merging this PR would make that removal harder. I mean we'd still have a |
@youknowriad I guess you are talking about the Hm, if you are planning to remove the That said—and answering your question about breaking changes—if you start with two things that allow to do (almost) the same thing, and then remove one of these things, to me, this is a breaking change anyway. At least from a user's perspective. Moving something from a dynamic inline block element to the Document sidebar is not just a re-design, in my opinion. And this is even more different on mobile... |
Hi @youknowriad, @mapk, it seems until now we did not remove the sidebar permalink editor. In that case, should we merge this PR or the plan is to still remove the component from the sidebar? |
I still think we should probably remove that component (the duplicate permalink UI above the title) but I'd like design confirmation @jasmussen @mapk @mtias |
The permalink UI being attached to the Title has not been very discoverable in practice. So I would think that this component should either be rethought entirely, or we should remove it entirely in favor of just the permalink editing happening in the sidebar, and in the pre-publish flow (where the permalink isn't yet, but it probably should be). Additionally there's the caveat of the Title becoming an actual block that you can remove entirely from a page, necessitating the ability to name the page and the permalink elsewhere (i.e. sidebar, on page creation, or in the editor header bar if we can fit it). |
Yep, removing it would be the future-forward path. The Title will be a block and so we'll need to add this permalink and title elsewhere in the UI. |
Closing as this component has been removed. |
PostPermalink
in@wordpress/editor
is not exported. We are developing a custom post title block and it would be handy to have this component exported instead of replicating it in our own codebase.Related: #3517