From 2d79ce2e93f68f64436d23f5eb7aa43a7326cbda Mon Sep 17 00:00:00 2001 From: sanchi-t Date: Wed, 24 Jan 2024 18:33:43 +0530 Subject: [PATCH] invite: Replace stream checkboxes list with stream pills. Introduce an input field with typeahead functionality, initially populated with the default streams for the organization. Fixes #26871. --- tools/test-js-with-node | 1 + web/src/invite.ts | 27 +++----- web/src/invite_stream_picker_pill.ts | 55 ++++++++++++++++ web/src/pill_typeahead.ts | 56 ++++++++++++++++- web/src/stream_data.ts | 28 ++------- web/src/stream_pill.ts | 47 ++++++++++---- web/styles/input_pill.css | 1 + web/styles/zulip.css | 20 ++---- web/templates/invite_user_modal.hbs | 29 +++------ web/tests/pill_typeahead.test.js | 93 ++++++++++++++++++++++++++++ web/tests/stream_data.test.js | 38 +++++++++++- web/tests/stream_pill.test.js | 37 ++++++++++- 12 files changed, 339 insertions(+), 93 deletions(-) create mode 100644 web/src/invite_stream_picker_pill.ts diff --git a/tools/test-js-with-node b/tools/test-js-with-node index d626c8747c..24f79f2be2 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -124,6 +124,7 @@ EXEMPT_FILES = make_set( "web/src/information_density.ts", "web/src/integration_url_modal.ts", "web/src/invite.ts", + "web/src/invite_stream_picker_pill.ts", "web/src/left_sidebar_navigation_area.ts", "web/src/left_sidebar_navigation_area_popovers.js", "web/src/lightbox.ts", diff --git a/web/src/invite.ts b/web/src/invite.ts index 350498934a..115acb3d6a 100644 --- a/web/src/invite.ts +++ b/web/src/invite.ts @@ -20,19 +20,21 @@ import * as dialog_widget from "./dialog_widget"; import * as email_pill from "./email_pill"; import {$t, $t_html} from "./i18n"; import * as input_pill from "./input_pill"; +import * as invite_stream_picker_pill from "./invite_stream_picker_pill"; import {page_params} from "./page_params"; import * as settings_config from "./settings_config"; import * as settings_data from "./settings_data"; import {current_user, realm} from "./state_data"; import * as stream_data from "./stream_data"; +import * as stream_pill from "./stream_pill"; import * as timerender from "./timerender"; import type {HTMLSelectOneElement} from "./types"; import * as ui_report from "./ui_report"; -import * as util from "./util"; let custom_expiration_time_input = 10; let custom_expiration_time_unit = "days"; let pills: email_pill.EmailPillWidget; +let stream_pill_widget: stream_pill.StreamPillWidget; function reset_error_messages(): void { $("#dialog_error").hide().text("").removeClass(common.status_classes); @@ -65,7 +67,7 @@ function get_common_invitation_data(): { expires_in = Number.parseFloat(raw_expires_in); } - const stream_ids: number[] = []; + let stream_ids: number[] = []; let include_realm_default_subscriptions = false; if ( $("#invite_select_default_streams").prop("checked") || @@ -73,10 +75,7 @@ function get_common_invitation_data(): { ) { include_realm_default_subscriptions = true; } else { - $("#invite-stream-checkboxes input:checked").each(function () { - const stream_id = Number.parseInt($(this).val()!, 10); - stream_ids.push(stream_id); - }); + stream_ids = stream_pill.get_stream_ids(stream_pill_widget); } assert(csrf_token !== undefined); @@ -248,12 +247,6 @@ function generate_multiuse_invite(): void { }); } -export function get_invite_streams(): stream_data.InviteStreamData[] { - const streams = stream_data.get_invite_stream_data(); - streams.sort((a, b) => util.strcmp(a.name, b.name)); - return streams; -} - function valid_to(time_valid: number): string { if (!time_valid) { return $t({defaultMessage: "Never expires"}); @@ -310,11 +303,9 @@ function set_streams_to_join_list_visibility(): void { realm_has_default_streams && $("input#invite_select_default_streams")[0]!.checked; if (hide_streams_list) { - $("#streams_to_add .invite-stream-controls").hide(); - $("#invite-stream-checkboxes").hide(); + $(".add_streams_container").hide(); } else { - $("#streams_to_add .invite-stream-controls").show(); - $("#invite-stream-checkboxes").show(); + $(".add_streams_container").show(); } } @@ -342,8 +333,6 @@ function open_invite_user_modal(e: JQuery.ClickEvent): void invite_as_options: settings_config.user_role_values, expires_in_options: settings_config.expires_in_values, time_choices: time_unit_choices, - streams: get_invite_streams(), - new_stream_announcements_stream: stream_data.get_new_stream_announcements_stream(), show_select_default_streams_option: stream_data.get_default_stream_ids().length !== 0, user_has_email_set: !settings_data.user_email_not_configured(), can_subscribe_other_users: settings_data.user_can_subscribe_other_users(), @@ -367,6 +356,8 @@ function open_invite_user_modal(e: JQuery.ClickEvent): void if (settings_data.user_can_subscribe_other_users()) { set_streams_to_join_list_visibility(); + const $stream_pill_container = $("#invite_streams_container .pill-container"); + stream_pill_widget = invite_stream_picker_pill.create($stream_pill_container); } $("#invite-user-modal").on("click", ".setup-tips-container .banner_content a", () => { diff --git a/web/src/invite_stream_picker_pill.ts b/web/src/invite_stream_picker_pill.ts new file mode 100644 index 0000000000..07a5116f5f --- /dev/null +++ b/web/src/invite_stream_picker_pill.ts @@ -0,0 +1,55 @@ +import * as input_pill from "./input_pill"; +import {set_up_stream} from "./pill_typeahead"; +import * as stream_data from "./stream_data"; +import * as stream_pill from "./stream_pill"; +import type {CombinedPillItem} from "./typeahead_helper"; + +type SetUpPillTypeaheadConfig = { + pill_widget: stream_pill.StreamPillWidget; + $pill_container: JQuery; +}; + +function create_item_from_stream_name( + stream_name: string, + current_items: CombinedPillItem[], +): input_pill.InputPillItem | undefined { + const stream_prefix_required = false; + const get_allowed_streams = stream_data.get_invite_stream_data; + const show_stream_sub_count = false; + return stream_pill.create_item_from_stream_name( + stream_name, + current_items, + stream_prefix_required, + get_allowed_streams, + show_stream_sub_count, + ); +} + +function set_up_pill_typeahead({pill_widget, $pill_container}: SetUpPillTypeaheadConfig): void { + const opts = { + help_on_empty_strings: true, + invite_streams: true, + }; + set_up_stream($pill_container.find(".input"), pill_widget, opts); +} + +export function add_default_stream_pills(pill_widget: stream_pill.StreamPillWidget): void { + const default_stream_ids = stream_data.get_default_stream_ids(); + for (const stream_id of default_stream_ids) { + const sub = stream_data.get_sub_by_id(stream_id); + if (sub) { + stream_pill.append_stream(sub, pill_widget, false); + } + } +} + +export function create($stream_pill_container: JQuery): stream_pill.StreamPillWidget { + const pill_widget = input_pill.create({ + $container: $stream_pill_container, + create_item_from_text: create_item_from_stream_name, + get_text_from_item: stream_pill.get_stream_name_from_item, + }); + add_default_stream_pills(pill_widget); + set_up_pill_typeahead({pill_widget, $pill_container: $stream_pill_container}); + return pill_widget; +} diff --git a/web/src/pill_typeahead.ts b/web/src/pill_typeahead.ts index 61ece4bc91..2d9e8b5dc7 100644 --- a/web/src/pill_typeahead.ts +++ b/web/src/pill_typeahead.ts @@ -6,7 +6,7 @@ import type {TypeaheadInputElement} from "./bootstrap_typeahead"; import * as people from "./people"; import type {User} from "./people"; import * as stream_pill from "./stream_pill"; -import type {StreamPillData} from "./stream_pill"; +import type {StreamPillData, StreamPillWidget} from "./stream_pill"; import * as typeahead_helper from "./typeahead_helper"; import type {CombinedPillContainer} from "./typeahead_helper"; import * as user_group_pill from "./user_group_pill"; @@ -72,6 +72,60 @@ export function set_up_user( }); } +export function set_up_stream( + $input: JQuery, + pills: StreamPillWidget, + opts: { + help_on_empty_strings?: boolean; + invite_streams?: boolean; + update_func?: () => void; + }, +): void { + const bootstrap_typeahead_input: TypeaheadInputElement = { + $element: $input, + type: "contenteditable", + }; + opts.help_on_empty_strings ||= false; + new Typeahead(bootstrap_typeahead_input, { + items: 5, + dropup: true, + helpOnEmptyStrings: true, + source(_query: string): StreamPillData[] { + return stream_pill.typeahead_source(pills, opts.invite_streams); + }, + highlighter_html(item: StreamPillData, _query: string): string { + return typeahead_helper.render_stream(item); + }, + matcher(item: StreamPillData, query: string): boolean { + query = query.toLowerCase(); + query = query.replaceAll("\u00A0", " "); + query = query.trim(); + if (query.startsWith("#")) { + query = query.slice(1); + } + return item.name.toLowerCase().includes(query); + }, + sorter(matches: StreamPillData[], query: string): StreamPillData[] { + const stream_matches: StreamPillData[] = []; + for (const match of matches) { + assert(match.type === "stream"); + stream_matches.push(match); + } + query = query.trim(); + if (query.startsWith("#")) { + query = query.slice(1); + } + return typeahead_helper.sort_streams(stream_matches, query); + }, + updater(item: StreamPillData, _query: string): undefined { + stream_pill.append_stream(item, pills, false); + $input.trigger("focus"); + opts.update_func?.(); + }, + stopAdvance: true, + }); +} + export function set_up_combined( $input: JQuery, pills: CombinedPillContainer, diff --git a/web/src/stream_data.ts b/web/src/stream_data.ts index f015f5ee10..49ab59fdb5 100644 --- a/web/src/stream_data.ts +++ b/web/src/stream_data.ts @@ -359,32 +359,14 @@ export function get_streams_for_user(user_id: number): { }; } -export function get_invite_stream_data(): InviteStreamData[] { - function get_data(sub: StreamSubscription): InviteStreamData { - return { - name: sub.name, - stream_id: sub.stream_id, - invite_only: sub.invite_only, - is_web_public: sub.is_web_public, - default_stream: default_stream_ids.has(sub.stream_id), - }; - } - +export function get_invite_stream_data(): StreamSubscription[] { const streams = []; - - // Invite users to all default streams... - for (const stream_id of default_stream_ids) { - const sub = sub_store.get(stream_id)!; - streams.push(get_data(sub)); - } - - // ...plus all your subscribed streams (avoiding repeats). - for (const sub of subscribed_subs()) { - if (!default_stream_ids.has(sub.stream_id)) { - streams.push(get_data(sub)); + const all_subs = get_unsorted_subs(); + for (const sub of all_subs) { + if (can_subscribe_others(sub)) { + streams.push(sub); } } - return streams; } diff --git a/web/src/stream_pill.ts b/web/src/stream_pill.ts index 88678b5416..6b1d70f5fe 100644 --- a/web/src/stream_pill.ts +++ b/web/src/stream_pill.ts @@ -11,7 +11,7 @@ export type StreamPill = { stream_name: string; }; -type StreamPillWidget = InputPillContainer; +export type StreamPillWidget = InputPillContainer; export type StreamPillData = StreamSubscription & {type: "stream"}; @@ -26,25 +26,40 @@ function format_stream_name_and_subscriber_count(sub: StreamSubscription): strin export function create_item_from_stream_name( stream_name: string, current_items: CombinedPillItem[], + stream_prefix_required = true, + get_allowed_streams: () => StreamSubscription[] = stream_data.get_unsorted_subs, + show_subscriber_count = true, ): InputPillItem | undefined { stream_name = stream_name.trim(); - if (!stream_name.startsWith("#")) { - return undefined; + if (stream_prefix_required) { + if (!stream_name.startsWith("#")) { + return undefined; + } + stream_name = stream_name.slice(1); } - stream_name = stream_name.slice(1); const sub = stream_data.get_sub(stream_name); if (!sub) { return undefined; } + const streams = get_allowed_streams(); + if (!streams.includes(sub)) { + return undefined; + } + if (current_items.some((item) => item.type === "stream" && item.stream_id === sub.stream_id)) { return undefined; } + let display_value = "#" + sub.name; + if (show_subscriber_count) { + display_value = format_stream_name_and_subscriber_count(sub); + } + return { type: "stream", - display_value: format_stream_name_and_subscriber_count(sub), + display_value, stream_id: sub.stream_id, stream_name: sub.name, }; @@ -67,33 +82,43 @@ export function get_user_ids(pill_widget: StreamPillWidget | CombinedPillContain export function append_stream( stream: StreamSubscription, - pill_widget: CombinedPillContainer, + pill_widget: StreamPillWidget | CombinedPillContainer, + show_subscriber_count = true, ): void { + let display_value = "#" + stream.name; + if (show_subscriber_count) { + display_value = format_stream_name_and_subscriber_count(stream); + } pill_widget.appendValidatedData({ type: "stream", - display_value: format_stream_name_and_subscriber_count(stream), + display_value, stream_id: stream.stream_id, stream_name: stream.name, }); pill_widget.clear_text(); } -export function get_stream_ids(pill_widget: CombinedPillContainer): number[] { +export function get_stream_ids(pill_widget: StreamPillWidget | CombinedPillContainer): number[] { const items = pill_widget.items(); return items.flatMap((item) => (item.type === "stream" ? item.stream_id : [])); } export function filter_taken_streams( items: StreamSubscription[], - pill_widget: CombinedPillContainer, + pill_widget: StreamPillWidget | CombinedPillContainer, ): StreamSubscription[] { const taken_stream_ids = get_stream_ids(pill_widget); items = items.filter((item) => !taken_stream_ids.includes(item.stream_id)); return items; } -export function typeahead_source(pill_widget: CombinedPillContainer): StreamPillData[] { - const potential_streams = stream_data.get_unsorted_subs(); +export function typeahead_source( + pill_widget: StreamPillWidget | CombinedPillContainer, + invite_streams?: boolean, +): StreamPillData[] { + const potential_streams = invite_streams + ? stream_data.get_invite_stream_data() + : stream_data.get_unsorted_subs(); return filter_taken_streams(potential_streams, pill_widget).map((stream) => ({ ...stream, type: "stream", diff --git a/web/styles/input_pill.css b/web/styles/input_pill.css index cbb31e6364..482681e460 100644 --- a/web/styles/input_pill.css +++ b/web/styles/input_pill.css @@ -180,6 +180,7 @@ } .add_subscribers_container .pill-container, +.add_streams_container .pill-container, .add_members_container .pill-container { width: 100%; background-color: hsl(0deg 0% 100%); diff --git a/web/styles/zulip.css b/web/styles/zulip.css index a103ffdd49..3084f47219 100644 --- a/web/styles/zulip.css +++ b/web/styles/zulip.css @@ -1586,21 +1586,13 @@ div.toggle_resolve_topic_spinner .loading_indicator_spinner { vertical-align: bottom; } -.invite-stream-controls button { - color: hsl(207.79deg 56.2% 52.55%); - background-color: transparent; +.add_streams_container { + display: inline-flex; + align-items: center; + width: 100%; - &:focus, - &:hover { - color: hsl(207.78deg 56.25% 37.65%); - text-decoration: underline; - background-color: transparent; - } -} - -#invite-stream-checkboxes { - & i.zulip-icon-globe { - font-size: 80%; + .pill-container { + padding: 4px 6px; } } diff --git a/web/templates/invite_user_modal.hbs b/web/templates/invite_user_modal.hbs index d106c01424..3af93231e8 100644 --- a/web/templates/invite_user_modal.hbs +++ b/web/templates/invite_user_modal.hbs @@ -67,33 +67,20 @@
{{#if show_select_default_streams_option}}
-
{{/if}} -
- | - -
-
- {{#each streams}} - - - {{/each}} +
+
+
+ {{~! Squash whitespace so that placeholder is displayed when empty. ~}} +
+
diff --git a/web/tests/pill_typeahead.test.js b/web/tests/pill_typeahead.test.js index afe544235e..4eafd2795b 100644 --- a/web/tests/pill_typeahead.test.js +++ b/web/tests/pill_typeahead.test.js @@ -224,6 +224,99 @@ run_test("set_up_user", ({mock_template, override, override_rewire}) => { assert.ok(input_pill_typeahead_called); }); +run_test("set_up_stream", ({mock_template, override, override_rewire}) => { + override_rewire(typeahead_helper, "render_stream", () => $fake_rendered_stream); + override_rewire(typeahead_helper, "sort_streams", ({streams}) => { + sort_streams_called = true; + return streams; + }); + mock_template("input_pill.hbs", true, (data, html) => { + assert.equal(typeof data.display_value, "string"); + assert.equal(typeof data.has_image, "boolean"); + return html; + }); + let input_pill_typeahead_called = false; + const $fake_input = $.create(".input"); + $fake_input.before = noop; + + const $container = $.create(".pill-container"); + $container.find = () => $fake_input; + + const $pill_widget = input_pill.create({ + $container, + create_item_from_text: noop, + get_text_from_item: noop, + }); + + let update_func_called = false; + function update_func() { + update_func_called = true; + } + + override(bootstrap_typeahead, "Typeahead", (input_element, config) => { + assert.equal(input_element.$element, $fake_input); + assert.equal(config.items, 5); + assert.ok(config.dropup); + assert.ok(config.stopAdvance); + + assert.equal(typeof config.source, "function"); + assert.equal(typeof config.highlighter_html, "function"); + assert.equal(typeof config.matcher, "function"); + assert.equal(typeof config.sorter, "function"); + assert.equal(typeof config.updater, "function"); + + // test queries + const stream_query = "#denmark"; + + (function test_highlighter() { + assert.equal( + config.highlighter_html(denmark_item, stream_query), + $fake_rendered_stream, + ); + })(); + + (function test_matcher() { + let result; + result = config.matcher(denmark_item, stream_query); + assert.ok(result); + result = config.matcher(sweden_item, stream_query); + assert.ok(!result); + })(); + + (function test_sorter() { + sort_streams_called = false; + config.sorter([denmark_item], stream_query); + assert.ok(sort_streams_called); + })(); + + (function test_source() { + const result = config.source(stream_query); + const stream_ids = result.map((stream) => stream.stream_id); + const expected_stream_ids = [denmark.stream_id, sweden.stream_id]; + assert.deepEqual(stream_ids, expected_stream_ids); + })(); + + (function test_updater() { + function number_of_pills() { + const pills = $pill_widget.items(); + return pills.length; + } + assert.equal(number_of_pills(), 0); + config.updater(denmark_item, stream_query); + assert.equal(number_of_pills(), 1); + + assert.ok(update_func_called); + })(); + + // input_pill_typeahead_called is set true if + // no exception occurs in pill_typeahead.set_up_user. + input_pill_typeahead_called = true; + }); + + pill_typeahead.set_up_stream($fake_input, $pill_widget, {update_func}); + assert.ok(input_pill_typeahead_called); +}); + run_test("set_up_combined", ({mock_template, override, override_rewire}) => { override_typeahead_helper(override_rewire); mock_template("input_pill.hbs", true, (data, html) => { diff --git a/web/tests/stream_data.test.js b/web/tests/stream_data.test.js index f20ffc102c..d7b14105a5 100644 --- a/web/tests/stream_data.test.js +++ b/web/tests/stream_data.test.js @@ -927,6 +927,9 @@ test("get_invite_stream_data", () => { }; people.init(); + people.add_active_user(me); + people.initialize_current_user(me.user_id); + current_user.is_admin = true; stream_data.add_sub(orie); stream_data.set_realm_default_streams([orie]); @@ -936,7 +939,7 @@ test("get_invite_stream_data", () => { name: "Orie", stream_id: 320, invite_only: false, - default_stream: true, + subscribed: true, is_web_public: false, }, ]; @@ -955,7 +958,38 @@ test("get_invite_stream_data", () => { name: "Inviter", stream_id: 25, invite_only: true, - default_stream: false, + subscribed: true, + is_web_public: false, + }); + assert.deepEqual(stream_data.get_invite_stream_data(), expected_list); + + // add unsubscribed private stream + const tokyo = { + name: "Tokyo", + stream_id: 12, + invite_only: true, + subscribed: false, + is_web_public: false, + }; + + stream_data.add_sub(tokyo); + assert.deepEqual(stream_data.get_invite_stream_data(), expected_list); + + const random = { + name: "Random", + stream_id: 34, + invite_only: false, + subscribed: false, + is_web_public: false, + }; + + stream_data.add_sub(random); + + expected_list.push({ + name: "Random", + stream_id: 34, + invite_only: false, + subscribed: false, is_web_public: false, }); assert.deepEqual(stream_data.get_invite_stream_data(), expected_list); diff --git a/web/tests/stream_pill.test.js b/web/tests/stream_pill.test.js index 6b4343baf3..5451804dab 100644 --- a/web/tests/stream_pill.test.js +++ b/web/tests/stream_pill.test.js @@ -4,8 +4,10 @@ const {strict: assert} = require("assert"); const {zrequire} = require("./lib/namespace"); const {run_test} = require("./lib/test"); +const {current_user} = require("./lib/zpage_params"); const peer_data = zrequire("peer_data"); +const people = zrequire("people"); const stream_data = zrequire("stream_data"); const stream_pill = zrequire("stream_pill"); @@ -19,6 +21,12 @@ const sweden = { name: "Sweden", subscribed: false, }; +const germany = { + stream_id: 103, + name: "Germany", + subscribed: false, + invite_only: true, +}; peer_data.set_subscribers(denmark.stream_id, [1, 2, 3, 77]); peer_data.set_subscribers(sweden.stream_id, [1, 2, 3, 4, 5]); @@ -36,14 +44,36 @@ const sweden_pill = { display_value: "translated: #Sweden: 5 users", }; -const subs = [denmark, sweden]; +const subs = [denmark, sweden, germany]; for (const sub of subs) { stream_data.add_sub(sub); } +const me = { + email: "me@example.com", + user_id: 5, + full_name: "Me Myself", +}; + +people.add_active_user(me); +people.initialize_current_user(me.user_id); + run_test("create_item", () => { - function test_create_item(stream_name, current_items, expected_item) { - const item = stream_pill.create_item_from_stream_name(stream_name, current_items); + current_user.user_id = me.user_id; + current_user.is_admin = true; + function test_create_item( + stream_name, + current_items, + expected_item, + stream_prefix_required = true, + get_allowed_streams = stream_data.get_unsorted_subs, + ) { + const item = stream_pill.create_item_from_stream_name( + stream_name, + current_items, + stream_prefix_required, + get_allowed_streams, + ); assert.deepEqual(item, expected_item); } @@ -51,6 +81,7 @@ run_test("create_item", () => { test_create_item("#sweden", [sweden_pill], undefined); test_create_item(" #sweden", [], sweden_pill); test_create_item("#test", [], undefined); + test_create_item("#germany", [], undefined, true, stream_data.get_invite_stream_data); }); run_test("get_stream_id", () => {