Fix summary & description fields deleting user's text (sometimes) after a commit #11345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sergiou87
commented
Jan 8, 2021
Comment on lines
+3581
to
+3584
| private async withIsCommitting( | ||
| repository: Repository, | ||
| fn: () => Promise<string | undefined> | ||
| ): Promise<boolean | undefined> { | ||
| fn: () => Promise<boolean> | ||
| ): Promise<boolean> { |
There was a problem hiding this comment.
Aside from renaming this function as @niik suggested, I also changed the return type to (hopefully) simplify the code, specially after moving things around.
sergiou87
commented
Jan 8, 2021
Comment on lines
+779
to
+782
| invalidationProps={{ | ||
| workingDirectory: this.props.workingDirectory, | ||
| isCommitting: this.props.isCommitting, | ||
| }} |
There was a problem hiding this comment.
After the changes in this PR, I noticed the checkboxes of the files in the change list stayed disabled. Putting the _refreshDirectory and refreshChangesSection inside the withIsCommitting block meant that the workingDirectory prop would change before isCommitting is set to false. And when that happens, it wouldn't trigger a render on the list.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #7251
Description
This PR tackles a problem where the component
CommitMessagecould clear the summary and description textfields a few seconds after the commit completed, when the user might already have written something for their next commit.The problem was
CommitMessagerelied on therepository.state.isCommittingattribute to enable/disable the textfields, but wouldn't clear the textfields until the commit + some additional tasks were finished. So the timeline would look something like this:repository.state.isCommitting = true(textfields are now disabled)repository.state.isCommitting = false(textfields are now enabled)This bug surfaces when step (4) takes too long, which is when the user might be preparing their next commit message, just to see how it gets deleted in step (5).
With this PR, the timeline of events looks like this:
repository.state.isCommitting = true(textfields are now disabled)repository.state.isCommitting = false(textfields are now enabled)Now the the textfields will be cleared right after enabling them again.
Release notes
Notes: [Fixed] Commit messages won't be cleared while the user is writing them.