Don't ask user about discarding message/losing changes if they haven't done anything in editor #1633

manuel12 · 2023-06-23T16:12:38Z

I made changes to fix #1540

I added a conditional to the relevant onCancel functions:

const editorEmpty =
        this.state.post.length === 0 && this.state.title.length === 0

if (editorEmpty) {
    this.minimize()
    posting.close()
    return
}

This way when discarding a thread, a private thread, editing or replying the dialog will only appear post or title have a lenght other than 0.

manuel12 · 2023-06-23T16:13:37Z

I noticed that there are also some polls that you can cancel and see the dialog but I'm not sure how to create a poll if you tell me how to test this I will also remove the dialog when they are empty.

rafalp · 2023-06-23T16:15:24Z

This is good first step, but what about attachments or initial value being quote of other reply?

rafalp · 2023-06-23T16:16:24Z

On thread pages there’s „add poll” button next to repky button.

manuel12 · 2023-06-23T16:58:55Z

On thread pages there’s „add poll” button next to repky button.

Thanks!

manuel12 · 2023-06-23T16:59:12Z

On thread pages there’s „add poll” button next to repky button.

I will make an update for this case too :)

manuel12 · 2023-06-24T14:02:30Z

@rafalp Hello I added a few updates. Now it should have removed discard changes text for polls where the user has not added any data, and quote replies where there is nothing in the editor aside from the initial quote.

manuel12 · 2023-06-24T14:41:34Z

@rafalp I've also added checks to make sure discard changes message is only skipped when there are no attachments, or in the case of private threads, when there are no attachments or recipients.

Hopefully this covers it all, if anything is missing don't hesitate to message :)

rafalp

I've left some notes.

rafalp · 2023-06-27T18:35:58Z

frontend/src/components/posting/edit.js

@@ -60,6 +60,8 @@ export default class extends Form {
  }

  loadSuccess = (data) => {
+    this.state.originalPost = data.post


Question: does this need to be a state var? Can't we put this into class property?

Sure thing will update.

rafalp · 2023-06-27T18:36:35Z

frontend/src/components/poll/form/index.js

+
+    // Nothing added to the poll so no changes to discard
+    const nothingAddedToPoll =
+      pollData.question === "" &&


Why use pollData instead of this.state... like other places do? TBH I like that approach better.

True that. Not need to overcomplicate with another variable. Will remove.

rafalp · 2023-06-27T18:37:15Z

frontend/src/components/poll/form/index.js

+    const nothingAddedToPoll =
+      pollData.question === "" &&
+      pollData.choices[0].label === "" &&
+      pollData.choices[1].label === "" &&


What if choices are undefined? Or if there are more than default 2?

Mmm. I think for this I will add a check in case the choices array's length is more than 0 -and if so iterate through the array checking if the labels are empty.

This is the way.

rafalp · 2023-06-27T18:38:32Z

frontend/src/components/posting/reply.js

+
+    // If only the quote text is on editor user didn't add anything
+    // so no changes to discard
+    const onlyQuoteTextInEditor =


Seems flaky but still good enough for first iteration.

I think for this I will add a quoteText class var where I will save the original replay and compare it to that is on the editor at the time of canceling.

manuel12 · 2023-06-27T22:04:42Z

Ok I will make an update with the required changes.

manuel12 · 2023-06-28T15:15:06Z

@rafalp Ok I've added changes. Please check again.

rafalp · 2023-06-28T19:27:20Z

frontend/src/components/posting/reply.js

+    // so no changes to discard
+    const onlyQuoteTextInEditor = this.state.post === this.quoteText
+
+    if (editorEmpty || onlyQuoteTextInEditor) {


Shouldn't this be &&? this.state.attachments.length === 1 will cause editorEmpty to be false, but if both post and quoteText are empty strings, onlyQuoteTextInEditor will be true?

I've updated the conditional to make it simpler. Now it checks if onlyQuoteTextInEditor and this.state.attachments === 0.

rafalp · 2023-06-28T19:28:16Z

Did you format the code with prettier? I think those multiline conditions should be wrapped in parenthesis by it ((...)) and I like them that way more.

manuel12 · 2023-06-29T17:54:21Z

Did you format the code with prettier? I think those multiline conditions should be wrapped in parenthesis by it ((...)) and I like them that way more.

For me it's removing the parenthesis on save.

rafalp · 2023-06-29T18:22:19Z

You are correct, I am wrapping those in !!(...) elsewhere in code to prevent it from doing this.

manuel12 · 2023-06-29T20:00:42Z

You are correct, I am wrapping those in !!(...) elsewhere in code to prevent it from doing this.

Ok, I added parenthesis around the conditionals.

rafalp · 2023-06-30T11:09:18Z

frontend/src/components/poll/form/index.js

@@ -67,6 +67,19 @@ export default class extends Form {
  onCancel = () => {
    let cancel = false

+    // Nothing added to the poll so no changes to discard


Common semantics for forms are calling them either "clean" (or "empty)" (no changes made by the user) or "dirty" (changes made by the user).

Can you update this variable to formEmpty?

rafalp · 2023-06-30T11:12:15Z

frontend/src/components/posting/start.js

@@ -88,6 +88,17 @@ export default class extends Form {
  }

  onCancel = () => {
+    const editorEmpty = !!(


Suggested change

const editorEmpty = !!(

const formEmpty = !!(

rafalp · 2023-06-30T11:12:32Z

frontend/src/components/posting/start-private.js

@@ -40,6 +40,17 @@ export default class extends Form {
  }

  onCancel = () => {
+    const editorEmpty = !!(


Suggested change

const editorEmpty = !!(

const formEmpty = !!(

rafalp · 2023-07-01T08:42:56Z

hljs is no longer minified. Same for zxcvbn.js? 🤔

manuel12 · 2023-07-01T15:52:10Z

hljs is no longer minified. Same for zxcvbn.js? 🤔

I was running npm run start before to build. Now I've ran npm run build for production and commited it.

coveralls · 2023-07-01T19:59:54Z

coverage: 98.563% (-0.009%) from 98.572% when pulling 548ad4b on manuel12:no-discarding-message-when-editor-empty into e3b0786 on rafalp:main.

rafalp · 2023-07-01T20:00:48Z

Please rebase and I'll merge this <3

…t's true

…ed to the poll

…of quote replies

Rebasing branch no-discarding-message-when-editor-empty with main branch changes.

and feature branch changes.

…all choices labels are empty.

manuel12 · 2023-07-03T15:15:19Z

@rafalp Please check.

I'm not so sure I should have all the commits repeated after the last push.

I also used force push after rebasing. I couldn't find a way to fix the git Push rejected: non fast-forward issue. So I don't know if this works or not.

In any case I checked and all my changes were relatively simple and on only a few files. Worst case scenario I can create a new branch that has all the current changes from origin and re-add my changes there and do another PR.

rafalp · 2023-07-08T20:15:48Z

Trick to rebase is that you pull main branch first to make sure it has latest changes. Then on your branch you do rebase -i main. For every conflict you change your files to fix the conflict, then git add them and do git rebase --continue. If you git commit during rebase you will mess up the git changes tree.

But at the end I'll squash your changes on merge, so those all commits will be rolled up into one on main branch.

manuel12 · 2023-07-10T10:13:04Z

Trick to rebase is that you pull main branch first to make sure it has latest changes. Then on your branch you do rebase -i main. For every conflict you change your files to fix the conflict, then git add them and do git rebase --continue. If you git commit during rebase you will mess up the git changes tree.

But at the end I'll squash your changes on merge, so those all commits will be rolled up into one on main branch.

Ok. Should I do anything more on my side or? :)

rafalp · 2023-07-11T17:24:52Z

Thank you!

manuel12 · 2023-07-11T22:42:17Z

Thank you!

Glad to help!

rafalp reviewed Jun 27, 2023

View reviewed changes

rafalp changed the title ~~[FIX] Don't ask user about discarding message/losing changes if they haven't done anything in editor~~ Don't ask user about discarding message/losing changes if they haven't done anything in editor Jun 28, 2023

rafalp added this to the 0.37 milestone Jun 28, 2023

rafalp reviewed Jun 28, 2023

View reviewed changes

rafalp reviewed Jun 30, 2023

View reviewed changes

rafalp assigned manuel12 Jul 1, 2023

rafalp added area: frontend This issue involves JavaScript, React.js and Node enhancement Improvement to existing feature or dev experience labels Jul 1, 2023

rafalp approved these changes Jul 1, 2023

View reviewed changes

manuel12 added 2 commits July 3, 2023 16:24

Added editorEmpty conditional and minimized, closed and returned if i…

2127869

…t's true

Added editorEmpty conditional and minimized, closed and returned if i…

c5084a1

…t's true

manuel12 added 19 commits July 3, 2023 16:24

Added editorEmpty conditional and minimized, closed and returned if i…

a383439

…t's true

Added editorEmpty conditional and minimized, closed and returned if i…

f76f832

…t's true

Added pollData const and conditional to check if nothing has been add…

c8680d7

…ed to the poll

Linked return statement to posting.close()

a9cf5b4

Added check to see if there is only the quote text in editor in case …

6e18eb8

…of quote replies

Linked return statement to posting.close()

addb57e

Linked return statement to this.close()

0906479

Added my changes to main branch files

491e4c8

Rebase commit.

f386364

Rebasing branch no-discarding-message-when-editor-empty with main branch changes.

Commit of misago.js file with both main branch

0e42d57

and feature branch changes.

Added check for choices array and also used Array.every to make sure …

5511750

…all choices labels are empty.

Static files commit

00fa6d9

Static file commit

e5aaa41

Added parenthesis around conditionals

c4e22f1

Added parenthesis around conditionals

07185ef

Static files commit

cc82629

Renamed editorEmpty to formEmpty

2e5dd33

Static files commit

b92e8c8

Created staticfiles production build

548ad4b

manuel12 force-pushed the no-discarding-message-when-editor-empty branch from 44f55f8 to 548ad4b Compare July 3, 2023 15:14

manuel12 requested a review from rafalp July 3, 2023 15:17

rafalp approved these changes Jul 11, 2023

View reviewed changes

rafalp merged commit d2cd85a into rafalp:main Jul 11, 2023
1 check passed

Don't ask user about discarding message/losing changes if they haven't done anything in editor #1633

Don't ask user about discarding message/losing changes if they haven't done anything in editor #1633

Conversation

manuel12 commented Jun 23, 2023

manuel12 commented Jun 23, 2023

rafalp commented Jun 23, 2023

rafalp commented Jun 23, 2023

manuel12 commented Jun 23, 2023

manuel12 commented Jun 23, 2023

manuel12 commented Jun 24, 2023

manuel12 commented Jun 24, 2023

rafalp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuel12 commented Jun 27, 2023

manuel12 commented Jun 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafalp commented Jun 28, 2023

manuel12 commented Jun 29, 2023

rafalp commented Jun 29, 2023

manuel12 commented Jun 29, 2023

rafalp Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafalp commented Jul 1, 2023

manuel12 commented Jul 1, 2023

coveralls commented Jul 1, 2023 • edited Loading

rafalp commented Jul 1, 2023

manuel12 commented Jul 3, 2023 • edited Loading

rafalp commented Jul 8, 2023

manuel12 commented Jul 10, 2023

rafalp commented Jul 11, 2023

manuel12 commented Jul 11, 2023

rafalp Jun 30, 2023 •

edited

Loading

coveralls commented Jul 1, 2023 •

edited

Loading

manuel12 commented Jul 3, 2023 •

edited

Loading