-
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
Edit Post: Fix pattern modal reopening when making the title empty again #55873
Conversation
Size Change: +845 B (0%) Total Size: 1.7 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.
Thanks for a nice fix! I was able to reproduce the issue and to verify that the PR really fixes it. I also checked that there's no regression related to bugs fixed in #54245.
/> | ||
</div> | ||
</Modal> | ||
); | ||
} | ||
|
||
export default function StartPageOptions() { | ||
const [ modalState, setModalState ] = useState( 'initial' ); |
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 nice that you've been able to reduce the three-state (initial
, open
, closed
) logic into just two states, "open", "closed". The code could be further simplified to use a single boolean value:
const [ isClosed, setClosed ] = useState( false );
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.
Makes sense, updated with 8127399 👍
The code matches very closely what I also did in another PR, so that's very reassuring #55117 (comment) 😄 Great to get this merged first, thanks! 👍 |
Apologies @talldan for causing merge conflicts in your PR 🙂 |
Fixes #55847
What?
This PR fixes an issue where after closing the pattern modal when opening a new page, if you make the post title empty again, the modal appears again.
79a073c74186e99fd84a7b8f48a6b898.mp4
Why?
This modal primarily consists of
StartPageOptions
and a childStartPageOptionsModal
component. Whether this modal is opened is managed by the childStartPageOptionsModal
component.Whether or not child
StartPageOptionsModal
components are rendered is controlled by theshouldOpenModal
variable. This variable contains the condition whether the post is empty or not (isCleanNewPost
).When you enter a post title and then empty the title, the
shouldOpenModal
variable changes fromfalse
totrue
, causing theStartPageOptionsModal
component to be rendered again.As a result, the
StartPageOptionsModal
component'smodalState
is initialized and the modal is displayed again.How?
I decided to manage whether the modal is closed or not using a higher-level component, the
StartPageOptions
component. This PR is related to #53673 which improved performance, but should not impact performance.Testing Instructions