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

settings: Auto save user group members on blur. #8716

Closed
wants to merge 5 commits into from

Conversation

shubham-padia
Copy link
Member

Fixes #8088.

  • Cancel button fades out if user removes the changes manually i.e without clicking the cancel button.
  • Clicking on cancel button discards the changes and the user group returns to its original state.
  • Suppose you've made a change to the name, clicking on the description edit or pill input of the same group does not save that name, but if you click
    any input that is not present in the current group, then that change is saved before allowing the user to edit the second group.
  • Solved: If user has added 1 user and tries to add another user (the 1st user is unsaved), then as typeahead is outside the input box, blur event is triggered resulting in the saving of 1st member which wasn't the intention.

saveonblur1

Changes saved if any other user group input clicked:
saveonblur2

Auto saves group members if focus moves from the input field in the
pill container.
Add do_not_blur func to not save changes when blur event's origin
is one of name/description/pill input from the current user group.
Changes in any of name/desc/members are saved together on blur from
any of the input field given do_not_blur is false.
Also replaces trash icon with "Delete" for the button.
Clicking the cancel button removes all the changes and the user
group returns back to the original state. Saved button is showed
once the changes are saved on blur.
@zulipbot
Copy link
Member

Hello @zulip/server-settings members, this pull request was labeled with the area: settings (admin/org) label, so you may want to check it out!

assert.equal(group_description, 'uranohoshi — Students at Uranohoshi Academy');
assert.equal(group_name_pills, 'uranohoshi');
assert.equal(group_name_display, 'uranohoshi');
assert.equal(group_description, 'Students at Uranohoshi Academy');
Copy link
Member Author

Choose a reason for hiding this comment

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

Earlier the entire h4 row was being checked. Changing the delete user group button from trash icon to Delete string resulted in:

assert.equal(group_description, 'uranohoshi — Students at Uranohoshi Academy translated: Delete');

Do let me know if this change needs to be reverted

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This seems totally fine.

@shubham-padia
Copy link
Member Author

@timabbott @rishig Please review.
Do let me know if any more info is needed regarding the behavior or the code changes :)

@timabbott
Copy link
Sponsor Member

This is excellent work @shubham-padia! Merged. It's rare to see such a large change that I don't have anything to complain about :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants