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

org settings: Change user joining invitation setting to dropdown. #9275

Closed

Conversation

shubhamdhama
Copy link
Member

Last followup of #9071 (comment)
GIF:
cool

@shubhamdhama
Copy link
Member Author

Feedback regarding strings required!
@timabbott @rishig FYI.

@rishig
Copy link
Member

rishig commented Apr 30, 2018

Great, thanks!

  • Let's flip the order of the two settings, so the email domain one comes second
  • Let's call the email domain one "Email domains of new users" rather than "Restrict email domains of new users".

For the text of the new settings, maybe:

Invitations
* No invitation required to join
* Anyone can invite new users 
* Only admins can invite new users

@timabbott
Copy link
Sponsor Member

I'm not sure I like how the last two read, in the context of reading what the current state is without clicking into the dropdown. Maybe:

Invitations for creating accounts:

  • Not required.
  • Required; any user can send invitations.
  • Required; only organization administrators can send invitations.

@rishig
Copy link
Member

rishig commented Apr 30, 2018

Seems reasonable. This is right below "Joining the organization", but we could also do
Invitations for joining the organization
as the label?

Styling notes for when this is implemented

  • No periods at the end (and no : at the end of the label)
  • "organization administrators" is "admins" in settings dropdowns.

@shubhamdhama
Copy link
Member Author

@rishig @timabbott Does this looks good?

screenshot from 2018-05-01 01-29-06

overview

@rishig
Copy link
Member

rishig commented May 1, 2018

Thanks @shubhamdhama! One more attempt; I think this is better than the ones so far :).

Are invitations required for joining the organization?
* No
* Yes, and anyone can send invitations
* Yes, and only admins can send invitations

@shubhamdhama
Copy link
Member Author

@rishig Here are various screenshots of the dropdown, let me know how this looks ow?

screenshot from 2018-05-02 21-45-36

screenshot from 2018-05-02 21-45-09

screenshot from 2018-05-02 21-44-12

nooption

@rishig
Copy link
Member

rishig commented May 2, 2018

lgtm, thanks! To make the second setting parallel to the first, we could go back to Restrict email domains of new users?, but with a question mark at the end.


var user_invite_restriction = $('#id_realm_user_invite_restriction').val();
if (user_invite_restriction === 'no_invite_required') {
opts.data.invite_required = false;
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think we want this to set invite_by_admins_only = false as well, right? Just from a general cleanlyness perspective.

This is a minor clean up of some contexts for admin pages
which we don't need anymore.
@shubhamdhama shubhamdhama force-pushed the settings-make-invite-dropdown branch 2 times, most recently from 97d530a to 6d28d58 Compare May 2, 2018 17:50
@shubhamdhama
Copy link
Member Author

@rishig @timabbott updated(waiting for tests to get passed)
screenshot from 2018-05-02 23-20-38

@timabbott
Copy link
Sponsor Member

@shubhamdhama looks great, though I'd put that text change for the second dropdown into its own commit.

This change with label text is done to make things similar the one for
realm_user_invite_restriction.
@shubhamdhama shubhamdhama force-pushed the settings-make-invite-dropdown branch from 6d28d58 to 391ffeb Compare May 2, 2018 18:05
@shubhamdhama
Copy link
Member Author

Ahh sorry, I'll keep this in mind!
(updated)

@timabbott
Copy link
Sponsor Member

Merged, after changing the "anyone" to "normal users"; I think it's important to distinguish users from "people on the internet", and also, we'll eventually want to talk about "normal users" to distinguish from guest users.

@timabbott timabbott closed this May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants