-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Update: Implement the new discussion panel design. #61357
Update: Implement the new discussion panel design. #61357
Conversation
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +569 B (+0.03%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
2a60d9a
to
2973c73
Compare
This is a valid issue and it will occur with the I've worked on resolving this independently in #61476. I think |
@tyxla Thanks, I'll take a look at your PR. To be clear I think we should do both;
|
f6dfb4b
to
c499802
Compare
Hi @jameskoster I applied the min-width to the popover and used the suggested labels. Thank you for the review and feedback let me know if there are any other updates we should do. |
Thanks @jorgefilipecosta, one small tweak; Since pages do not support pingbacks, the button label can just be 'Open' or 'Closed'. Currently if you set discussion to open while editing a page the button says 'Comments only' which might suggest there are other options. Otherwise this looks good :) |
63de2aa
to
0fae69b
Compare
Hi @jameskoster, I applied the suggestion and made the labels dependent on the post type support for trackbacks and comments. Let me know if it has the green light now :) |
b8449ce
to
fe0aaa5
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 pushed a small tweak to adjust spacing, and make the documentation link help text for the checkbox control which seems semantically appropriate.
The mis-alignment of text between radios/checkboxes is bothersome, but that's something to fix at the component level.
const postSummarySection = page.getByRole( 'checkbox', { | ||
name: 'Stick to the top of the blog', | ||
} ); | ||
|
||
await expect( postExcerptPanel ).toBeVisible(); | ||
await expect( postFeaturedImagePanel ).toBeVisible(); | ||
await expect( postSummarySection ).toBeVisible(); | ||
await expect( postDiscussionPanel ).toHaveCount( 1 ); |
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.
It's recommended to use explicit assertions. See - https://github.com/WordPress/gutenberg/blob/trunk/docs/contributors/code/e2e/README.md#make-explicit-assertions.
@@ -147,5 +150,6 @@ test.describe( 'Sidebar', () => { | |||
await expect( postExcerptPanel ).toBeHidden(); | |||
await expect( postFeaturedImagePanel ).toBeHidden(); | |||
await expect( postSummarySection ).toBeHidden(); | |||
await expect( postDiscussionPanel ).toHaveCount( 0 ); |
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.
Same here.
Implements the new discussion panel design proposed at: #59689 (comment)
cc: @jameskoster
Screenshots
Testing Instructions
Tested the discussion panel options and verified they work as expected and are correctly saved.