Skip to content

Conversation

johnsimons
Copy link
Member

@johnsimons johnsimons commented Apr 30, 2025

The edit and retry dialog was suppose to show a "reset body" back to the original button.
But unfortunately the logic had a few bugs which cuased the button to not always be shown.

This PR fixes that issue as well as brings the button to the top of the dialog to be more inline with current design in other areas.

@johnsimons johnsimons self-assigned this Apr 30, 2025
const newValue = value.replaceAll(regExToPruneLineEndings, "");
localMessage.value.isBodyChanged = newValue !== uneditedMessageBody.value.replaceAll(regExToPruneLineEndings, "");
localMessage.value.isBodyEmpty = newValue === "";
}, 1000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these values affect the save? What happens during the second that you're waiting for the update to happen? Why is the debounce required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Becasue we don't want to fire this callback on every keystroke. The callback has a few things happening that can be expensive.
The worse it happens is that we won't see the "reset" button for 1 second.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as mentioned in our call, this should be removed for the isBodyEmpty check, retained for the isBodyChanged

Copy link
Contributor

@PhilBastian PhilBastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you provide a more detailed description of what the issue was that this is fixing? Difficult to get a handle on it from just the code...

@johnsimons
Copy link
Member Author

@PhilBastian I have updated the description

@johnsimons johnsimons requested a review from PhilBastian May 1, 2025 08:08
@johnsimons johnsimons force-pushed the john/editandreplay branch from e4c8dfc to 12c61ca Compare May 2, 2025 00:43
@johnsimons johnsimons enabled auto-merge May 2, 2025 00:51
@johnsimons johnsimons force-pushed the john/editandreplay branch from 12c61ca to 648cacf Compare May 2, 2025 00:52
@johnsimons johnsimons merged commit 5b1c6b0 into master May 2, 2025
5 checks passed
@johnsimons johnsimons deleted the john/editandreplay branch May 2, 2025 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants