-
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
Fix syncing of publish date between publish and post status panel #62165
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. |
startOfWeek: getSettings().l10n.startOfWeek, | ||
onChange, | ||
currentDate: isCompact ? undefined : currentDate, | ||
currentTime: isCompact ? currentDate : undefined, |
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 not clear to me why isCompact impacts currentData prop?
Do we want the date to be "controlled" outside the entity in the case of the post status. What's happening exactly here?
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.
TimePicker component has different name for the currentDate
and is currentTime
.
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.
Naming around the TimePicker
component is confusing.
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.
ah ok then this fix makes sense :P
Both of these components should just use value onChange to be honest.
Size Change: +25 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
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.
The fix works as expected ✅
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.
Changes look good to me.
I've noticed this component is also used in the post date block, which I've also tested and works as expected.
Gravacao.do.ecra.2024-05-31.as.10.00.29.mov
I see this is blocked by the performance job, which is failing in other PRs as well (different test suite: post-editor, site-editor, front-end-classic-theme). Tested locally with the lastest wp-env (to detect if it was a change in core) but I cannot reproduce. |
b3ff7af
to
4ce0884
Compare
Flaky tests detected in 4ce0884. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9316524086
|
…rdPress#62165) Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org>
…rdPress#62165) Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org>
What?
Fixes: #62150
In this PR I missed testing properly the syncing in both directions between the publish date and post status panel, when we update the publish date. This is the fix for that.
Testing Instructions
scheduled
status