-
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
Add a label to the URL field in the Publishing Flow panel #9536
Conversation
<p className="post-publish-panel__postpublish-subheader"> | ||
<strong>{ __( 'What’s next?' ) }</strong> | ||
</p> | ||
<label htmlFor={ id } className="post-publish-panel__postpublish-link-label"> |
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.
Can we use Text Control component to simplify this logic? (it automatically generates the ids).
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.
Hi @afercia thank you for looking into this issue. I did some tests, and things worked correctly 👍 From the code point view the changes look good, as a possible improvement we may check if Text Control component can be used here to simplify things and promote reusability.
Hi @jorgefilipecosta, thanks. Updated to use One thing I don't like so much in the This could be confusing for users, especially when the clickable area is very large. Should go in a separate issue though. Note: |
Looks like there are some conflicts on this. I am approving design wise but we would need to get those resolve, thanks! |
d21eb39
to
58e8b3f
Compare
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.
I rebased this PR (resolving the conflict) so the local tests take into account changes occurred in parallel.
The tests went well and the code looks good to me. Thank you for iterating on this changes 👍
Thank you Jorge. |
Should we consider URL over address? How does address scale for all situations? I'm not debating having the word as much as having some copy iterations. |
@karmatosed I also wonder if it's even better to switch over to "Permalink" since that's a standard and familiar WP term already used in a few places. |
Description
The Post URL field in the Publishing Flow panel is unlabelled. This PR adds a visible label. Also simplifies a bit the text for the Scheduled posts. I know there are pending proposal to improve and add features to the publishing flow so this part will be very likely improved and iterated. The most important thing in this PR is the input field label.
Screenshots
Before:
After:
Note 1:
the sentence "The post address will be:" is not appropriate anyways, as a post can be of different types: post, page, or a custom post type. To keep things simple I've just removed it in favor of "What’s next?".
Note 2:
since the URL field is
readonly
, I'd suggest to communicate to users it can't be edited. Usually in WordPress this is done by using a light gray background. Something like in the screenshot below or a lighter gray could work:Fixes #9535