Only enable the save changes button when there are changes to submit

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: daleharvey, Assigned: daleharvey)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

45 bytes, text/x-github-pull-request
Details | Review | Splinter Review
Comment hidden (empty)

Updated

3 years ago
Assignee: nobody → robert.sajdok

Comment 1

3 years ago
Created attachment 8653570 [details] [review]
Proposed fix
Attachment #8653570 - Flags: review?(dale)
(Assignee)

Comment 2

3 years ago
Comment on attachment 8653570 [details] [review]
Proposed fix

This wont quite work as expected, there is also the status selector (and in future there will likely be more), we need to disable the submit button only if there are no changes to the form.

With this patch if the user types, changes the status, then deletes that text it will disable the submit button and not let them change the status alone.
Attachment #8653570 - Flags: review?(dale)

Updated

3 years ago
Attachment #8653570 - Flags: review?(dale)
(Assignee)

Comment 3

3 years ago
Comment on attachment 8653570 [details] [review]
Proposed fix

So apologies but this wont quite work either. If the user changes the bug status then changes it back to the original there are no changes to send so the submit should be disabled. I also just realised we have a bug where the user cant assign themselves without a comment

I think we should do something like

function formInput() { 
  var enableSubmit = comment.value && 
    statusSelect.value !== currentStatus
  ... do the submit button
}

form.addEventListener('change', formInput);
// We dont get change events while the user is typing, so also need input event
form.addEventListener('input', formInput);

This would probably need debouncing so it only runs every X(100?)ms so it doesnt intefere with the users typing.

It should also be possible to share the code with the submission that has to figure out what has changed in order to submit it (https://github.com/rsajdok/bzlite/blob/bug%231198695/lib/views/bug-comments.js#L75) 

So the code could look something like:

function formInput() { 
  var changes = getChangedInput();
  var enabled = Object.keys(changes).length > 0; 

Sorry this is being a bit of a hassle
Attachment #8653570 - Flags: review?(dale)

Comment 4

3 years ago
(In reply to Dale Harvey (:daleharvey) from comment #3)
> Comment on attachment 8653570 [details] [review]
> Proposed fix
> 
> So apologies but this wont quite work either. 
> Sorry this is being a bit of a hassle
From my perspective the description of this bug was not prepared very well, no offence. It should be, for example: "disable, enable 'save changes' depending on changes on the whole page" 
I focused only on changes in comments box as it is described in the description of this bug.
(Assignee)

Comment 5

3 years ago
Yup apologies, I could have done a better job of explaining the bug
(Assignee)

Updated

3 years ago
Summary: If the user enters text into the comments box and then deletes it, the ‘save changes’ button should be disabled again. → Only enable the save changes button when there are changes to submit
(Assignee)

Updated

3 years ago
Assignee: robert.sajdok → dale
(Assignee)

Comment 6

3 years ago
Hey Robert, going to steal this and switch it to share the code that submits the bug so we dont need to worry about handling this for every input we add
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.