Skip to content
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

[FIX] Hightlights validation on Account Preferences page #19902

Merged
merged 5 commits into from
Dec 22, 2020

Conversation

aKn1ghtOut
Copy link
Contributor

@aKn1ghtOut aKn1ghtOut commented Dec 18, 2020

Checklist

  • I have read the Contributing Guide
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Proposed changes

This PR fixes two issues in the account settings "preferences" panel.
Once set, the "Highlighted Words" setting cannot be reset to an empty string. This was fixed by changing the string validation from checking the length to checking the type of variable.
Secondly, it tracks the changes to correctly identify if changes after the last "save changes" action have been made, using an "updates" state variable, instead of just comparing against the initialValue that does not change on clicking "save changes".

Issue(s)

Fixes issue #19896

Steps to test or reproduce

Reproduce in order.

  • Reload Webpage.
  • In Highlights, start editing the "Highlighted words" TextArea. [ Default value should now be "klt" ]
  • Delete all contents of the textarea. [ "Save Changes" correctly activates ]
  • Click "Save Changes" [ Shows error "[Undefined]" ]

Further comments

The first issue will affect other settings pages too. There are two solutions I considered to this: The one implemented here or simply reloading the webpage on clicking "Save Changes". The project should pick one approach and that should be implemented for all the settings pages.

@dougfabris dougfabris changed the title [FIX] Fixes inaccurate "Save Changes" activation and "highlights" validation on Account Preferences page [FIX] Hightlights validation on Account Preferences page Dec 21, 2020
Douglas Fabris added 2 commits December 21, 2020 18:59
It's not necessary to add another state to check changes
@dougfabris dougfabris linked an issue Dec 21, 2020 that may be closed by this pull request
@aKn1ghtOut
Copy link
Contributor Author

@dougfabris should I open a separate issue for the other issue that occurs because of no page refresh?

@dougfabris
Copy link
Member

dougfabris commented Dec 21, 2020

@aKn1ghtOut the fix should properly solve both now, can you check? btw thanks for helping us =)

@dougfabris dougfabris added this to the 3.10.0 milestone Dec 21, 2020
@aKn1ghtOut
Copy link
Contributor Author

Just checked. The "first issue" described above is still there. I do however doubt that it may be out of scope for the Issue being referenced here since it would warrant a uniform approach to solve it on all settings pages.
Can you have a look please?
Hope to be more useful in the future :)

@aKn1ghtOut
Copy link
Contributor Author

I just opened #19927 to allow more space for that bug specifically. If a different issue seems unnecessary, it can be closed and maybe, the discussion could be done on this PR itself.

@dougfabris
Copy link
Member

@aKn1ghtOut Ok thanks. I'll fix the description and check the other issue separately!

@dougfabris dougfabris requested a review from ggazzo December 21, 2020 23:33
@ggazzo ggazzo merged commit 2387412 into RocketChat:develop Dec 22, 2020
@sampaiodiego sampaiodiego mentioned this pull request Dec 29, 2020
@aKn1ghtOut aKn1ghtOut deleted the fix19378 branch January 7, 2021 17:07
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.

Match Failed [400]
3 participants