Improve invite modal to include pills and dropdown.

[shubhamdhama]

Current commits can be merged if they look great, I'll be adding further work soon.

[showell]

nitpick: I prefer a style where you can read the code in the order it happens:

var data = {stream: exports.get_invite_streams()};
var html = template.render('invite_subscription', data);
$('#streams_to_add').html(html)

[shubhamdhama]

yeah, this is way much better, just confused as once I was asked to not prefer variable if used only once, but it makes sense here to use otherwise the line becomes long... so changing it to this

[shubhamdhama]

Currently, it isn't impressive look wise and we may want a replacement of "Check all and deselect all" so what they should be?

For default streams:

[shubhamdhama]

#7998 can be closed now

Few other feedbacks required:

• How should streams be sorted in typeahead, currently it's a normal sort i.e. Capital letter sorted first, then lowercase and so on, so should I sort them irrespective of case? Also do we want to keep public streams first and then private or mixed form is fine?
• Next step would be to show "Lock" icon before private streams?
• "Add all streams" and "reset" makes sense to be placed somewhere?

[shubhamdhama]

(for me) TODO: fix for

[timabbott]

Hmm, I thought I'd replied to your questions. I think case-insensitive alphabetical sort is fine to start. And yeah, lock icon for private streams is good.
Let me know when you have this rebased and I'll take a look at the code.

[shubhamdhama]

@timabbott I've rebased the PR and I think first four commits are mergeable.

[timabbott]

Two problems with the first commit:

• The layout for the dropdown is off -- the label should be in line with the dropdown, not like this
• The default value should be "Regular users", not administrators, and we should probably call it "Members" (@rishig assuming we're planning to refactor terminology to that to contrast with "Guests"). I'd call the value "member" too.

[rishig]

yeah, Members sounds good. I would also make it "Administrators" rather than "Organization administrators".
(Alternatively, "Organization administrators" and "Organization members".)

[timabbott]

The second commit also doesn't work for me:

I think the bug is this:

• var data = {stream: exports.get_invite_streams()};

should be streams:.

I merged commits 3 and 4.

[shubhamdhama]

Okay, I'm updating your second point.
For styling one, you suggested that Max will do the styling work... so I think I should go for styling, he can work later.

[timabbott]

Yeah, on the styling front, I think it's worth your doing the minimal positioning work if it's quick to keep things mergable (i.e. not looking broken); that way we can merge the OK-looking version and then Max can restyle it to look great. Having a window where things are broken in master is difficult to manage.

[shubhamdhama]

Updated the first two commits. Working on improving next two.

[shubhamdhama]

I've updated the PR with following changes:

• Changed the stream_pill to invite_stream_pill as first I was thinking to make stream_pills generalized but after much efforts, I think it's better to create separate pills logic.
• Fixed some CSS issues with stream pills like placeholder text will only appear input box is empty as showing it always makes it wrapping to next line which is because of the small size of invite modal.
• Streams are sorted irrespective of the case of letters.

For icons I was thinking to do that in separate PR as for that I think I've to do a little refactoring in input_pills.js for custom pill HTML.
So these 4 commits are ready to be reviewed.

[timabbott]

I merged the first two commits; I believe that the dropdown change unblocks what we need for the "Guest users" feature, so we can consider that feature no longer blocked on finishing this.

I'm holding off on the stream pills mostly because I want to talk with @rishig about a use case that the pills don't support well (namely, easily seeing what streams a user isn't being added to as part of deciding how to set things up). I haven't figured out how big of a problem that is, but being able to see which streams were not currently included was a feature I found myself appreciating when setting up an organization for a hackathon this weekend.

[zulipbot]

Heads up @shubhamdhama, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

[rishig]

hm, interesting thought. For large realms (100+ streams) it seems like not much value, but I can see the benefit for a realm with 10 streams.

I could imagine putting
Not subscribed to: #general, #help, #random
in gray at the bottom of the input box, if it's fewer than 10 streams they are being not-subscribed to, or something like that.

[shubhamdhama]

(okay, let me know when we have reached the final consensus)
my personal feedback for this is that checkboxes were easier to use than input pills.
Maybe we can have a long dropdown which is always opened and just scroll through it to choose a stream along with text input, just like what others do here.

[zulipbot]

Heads up @shubhamdhama, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

[shubhamdhama]

Closing this as two commits are merged and as a reference of pills work and also I'm going to work on the new idea suggested here https://chat.zulip.org/#narrow/stream/6-frontend/subject/Invite.20Modal/near/638686

[rishig]

This question is independent of whether we have pills for the streams list. I do still think some version of this is a good idea, though may have to wait for Tim and I to be able to talk about it.