Group invalid inputs (#32647)

* css: Extract invalid input outline and shadow colors to variables.

We will use these colors in future for input pills and it would be
convenient to have them in a variable.

* group_setting_pill: Show outline on invalid input.

We could have manipulate the class directly from user_group_pill. But
it felt better to have `show_outline_on_invalid_input` as a param for
the input_pill prototype since we can have a consistent error state for
other pill input boxes if we want to.

* input_pill: Widget should not show as pending after pill creation.

* group_settings: Disable save changes button if pill widget is pending.

This will disable the button for group, realm and stream group settings.

* user_group_create: Don't go to next step with pending group widget.

We just show the red outline and shaking animation as an indication that
a group widget setting is pending when the user tries to go to the add
members step.

* stream_create: Don't go to next step with pending group widget.

Fixes #32113.
We just show the red outline and shaking animation as an indication that
a group widget setting is pending when the user tries to go to the add
subscribers step.
This commit is contained in:
Shubham Padia
2024-12-11 06:27:48 +05:30
committed by GitHub
parent 34d6d3948e
commit f147e4dd4a
10 changed files with 94 additions and 17 deletions

View File

@@ -139,6 +139,7 @@ export function create_pills(
setting_name,
setting_type,
},
show_outline_on_invalid_input: true,
});
return pill_widget;
}

View File

@@ -35,6 +35,7 @@ type InputPillCreateOptions<ItemType> = {
all_pills: InputPill<ItemType>[],
remove_pill: (pill: HTMLElement) => void,
) => void;
show_outline_on_invalid_input?: boolean;
};
export type InputPill<ItemType> = {
@@ -58,6 +59,7 @@ type InputPillStore<ItemType> = {
createPillonPaste?: () => void;
split_text_on_comma: boolean;
convert_to_pill_on_enter: boolean;
show_outline_on_invalid_input: boolean;
};
// These are the functions that are exposed to other modules.
@@ -98,6 +100,7 @@ export function create<ItemType extends {type: string}>(
convert_to_pill_on_enter: opts.convert_to_pill_on_enter ?? true,
generate_pill_html: opts.generate_pill_html,
on_pill_exit: opts.on_pill_exit,
show_outline_on_invalid_input: opts.show_outline_on_invalid_input ?? false,
};
// a dictionary of internal functions. Some of these are exposed as well,
@@ -133,12 +136,14 @@ export function create<ItemType extends {type: string}>(
create_item(text: string) {
const existing_items = funcs.items();
const item = store.create_item_from_text(text, existing_items, store.pill_config);
if (!item) {
store.$input.addClass("shake");
if (store.show_outline_on_invalid_input) {
store.$parent.addClass("invalid");
}
return undefined;
}
return item;
},
@@ -161,6 +166,15 @@ export function create<ItemType extends {type: string}>(
store.pills.push(payload);
store.$input.before(payload.$element);
if (store.show_outline_on_invalid_input && store.$parent.hasClass("invalid")) {
store.$parent.removeClass("invalid");
}
// If we check is_pending just after adding a pill, the
// text is still present until further input, so we
// manually clear it here.
this.clear_text();
if (store.onPillCreate !== undefined) {
store.onPillCreate();
}
@@ -359,6 +373,13 @@ export function create<ItemType extends {type: string}>(
// the hook receives the updated text content of the input unlike the "keydown"
// event which does not have the updated text content.
store.$parent.on("input", ".input", () => {
if (
store.show_outline_on_invalid_input &&
funcs.value(store.$input[0]!).length === 0 &&
store.$parent.hasClass("invalid")
) {
store.$parent.removeClass("invalid");
}
store.onTextInputHook?.();
});

View File

@@ -1395,13 +1395,16 @@ function should_disable_save_button_for_group_settings(settings: string[]): bool
);
}
assert(group_setting_config !== undefined);
if (group_setting_config.allow_nobody_group) {
continue;
}
const pill_widget = get_group_setting_widget(setting_name);
assert(pill_widget !== null);
if (pill_widget.is_pending()) {
return true;
}
if (group_setting_config.allow_nobody_group) {
continue;
}
const setting_value = get_group_setting_widget_value(pill_widget);
const nobody_group = user_groups.get_user_group_from_name("role:nobody")!;
if (setting_value === nobody_group.id) {
@@ -1595,6 +1598,9 @@ export function create_group_setting_widget({
if (group !== undefined) {
set_group_setting_widget_value(pill_widget, group[setting_name]);
pill_widget.onTextInputHook(() => {
save_discard_group_widget_status_handler($("#group_permission_settings"), group);
});
pill_widget.onPillCreate(() => {
save_discard_group_widget_status_handler($("#group_permission_settings"), group);
});
@@ -1668,6 +1674,12 @@ export function create_realm_group_setting_widget({
const $save_discard_widget_container = $(`#id_realm_${CSS.escape(setting_name)}`).closest(
".settings-subsection-parent",
);
pill_widget.onTextInputHook(() => {
if (pill_update_callback !== undefined) {
pill_update_callback();
}
save_discard_realm_settings_widget_status_handler($save_discard_widget_container);
});
pill_widget.onPillCreate(() => {
if (pill_update_callback !== undefined) {
pill_update_callback();
@@ -1712,6 +1724,9 @@ export function create_stream_group_setting_widget({
const $edit_container = stream_settings_containers.get_edit_container(sub);
const $subsection = $edit_container.find(".advanced-configurations-container");
pill_widget.onTextInputHook(() => {
save_discard_stream_settings_widget_status_handler($subsection, sub);
});
pill_widget.onPillCreate(() => {
save_discard_stream_settings_widget_status_handler($subsection, sub);
});

View File

@@ -70,6 +70,11 @@ export function maybe_update_error_message(): void {
}
}
const group_setting_widget_map = new Map<string, GroupSettingPillContainer | null>([
["can_administer_channel_group", null],
["can_remove_subscribers_group", null],
]);
class StreamSubscriptionError {
report_no_subs_to_stream(): void {
$("#stream_subscription_error").text(
@@ -200,7 +205,23 @@ $("body").on("click", ".settings-sticky-footer #stream_creation_go_to_subscriber
let invite_only = false;
let is_web_public = false;
if (is_stream_name_valid) {
let is_any_stream_group_widget_pending = false;
const permission_settings = Object.keys(realm.server_supported_permission_settings.stream);
for (const setting_name of permission_settings) {
const widget = group_setting_widget_map.get(setting_name);
assert(widget !== undefined);
assert(widget !== null);
if (widget.is_pending()) {
is_any_stream_group_widget_pending = true;
// We are not appending any value here, but instead this is
// a proxy to invoke the error state for a group widget
// that would usually get triggered on clicking enter.
widget.appendValue(widget.getCurrentText()!);
break;
}
}
if (is_stream_name_valid && !is_any_stream_group_widget_pending) {
if (privacy_type === "invite-only" || privacy_type === "invite-only-public-history") {
invite_only = true;
} else if (privacy_type === "web-public") {
@@ -520,6 +541,7 @@ function set_up_group_setting_widgets(): void {
$pill_container: $("#id_new_" + setting_name),
setting_name: stream_permission_group_settings_schema.parse(setting_name),
});
group_setting_widget_map.set(setting_name, group_setting_widgets[setting_name]);
}
}

View File

@@ -108,7 +108,23 @@ $("body").on("click", ".settings-sticky-footer #user_group_go_to_members", (e) =
const group_name = $<HTMLInputElement>("input#create_user_group_name").val()!.trim();
const is_user_group_name_valid = user_group_name_error.validate_for_submit(group_name);
if (is_user_group_name_valid) {
let is_any_group_widget_pending = false;
const permission_settings = Object.keys(realm.server_supported_permission_settings.group);
for (const setting_name of permission_settings) {
const widget = group_setting_widget_map.get(setting_name);
assert(widget !== undefined);
assert(widget !== null);
if (widget.is_pending()) {
is_any_group_widget_pending = true;
// We are not appending any value here, but instead this is
// a proxy to invoke the error state for a group widget
// that would usually get triggered on clicking enter.
widget.appendValue(widget.getCurrentText()!);
break;
}
}
if (is_user_group_name_valid && !is_any_group_widget_pending) {
user_group_components.show_user_group_settings_pane.create_user_group(
"user_group_members_container",
group_name,

View File

@@ -686,6 +686,8 @@
--color-background-animated-button: hsl(0deg 0% 90%);
--color-animated-button-text: hsl(0deg 0% 0%);
--color-background-animated-button-hover: hsl(240deg 96% 68%);
--color-invalid-input-border: hsl(3deg 57% 33%);
--color-invalid-input-box-shadow: 0 0 2px hsl(3deg 57% 33%);
/* Recent view */
--color-border-recent-view-row: hsl(0deg 0% 87%);
@@ -1443,6 +1445,8 @@
--color-background-animated-button: hsl(211deg 29% 14%);
--color-animated-button-text: hsl(0deg 0% 100%);
--color-background-animated-button-hover: hsl(240deg 96% 68%);
--color-invalid-input-border: hsl(3deg 73% 74%);
--color-invalid-input-box-shadow: 0 0 2px hsl(3deg 73% 74%);
/* Recent view */
--color-border-recent-view-row: hsl(0deg 0% 0% / 20%);

View File

@@ -329,8 +329,8 @@
&:has(.new_message_textarea.invalid),
&:has(.new_message_textarea.invalid:focus) {
border-color: hsl(3deg 57% 33%);
box-shadow: 0 0 2px hsl(3deg 57% 33%);
border-color: var(--color-invalid-input-border);
box-shadow: var(--color-invalid-input-box-shadow);
}
}

View File

@@ -273,14 +273,6 @@
}
}
#message-content-container {
&:has(textarea.new_message_textarea.invalid),
&:has(textarea.new_message_textarea.invalid:focus) {
border-color: hsl(3deg 73% 74%);
box-shadow: 0 0 2px hsl(3deg 73% 74%);
}
}
#stream-actions-menu-popover .sp-container {
background-color: transparent;

View File

@@ -177,6 +177,11 @@
display: none;
}
}
&.invalid {
border-color: var(--color-invalid-input-border);
box-shadow: var(--color-invalid-input-box-shadow);
}
}
#compose-direct-recipient .pill-container {

View File

@@ -60,6 +60,7 @@ run_test("basics", ({mock_template}) => {
};
widget.appendValidatedData(item);
assert.ok(!widget.is_pending());
assert.ok(inserted_before);
assert.deepEqual(widget.items(), [item]);