From bc89d9689384a8dbfc1d33fb2cecc063339598e2 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Mon, 19 May 2025 15:43:21 +0530 Subject: [PATCH] stream-settings: Fix duplicate IDs. This commit fixes the use of "stream_permission_settings" as ID for "Channel permissions" subsection in both stream edit and creation UI, which was not correct since ID should be unique. To fix this ID was removed from the element and following changes are done - - $("#stream_settings") element is now used as a container for live update functions for stream edit UI. - "stream-permissions" class is used to access the element instead of ID. - Advanced configurations container also had "stream-permissions" class before, and that was removed in this commit so that "Channel permissions" container can be identified uniquely and thus some CSS changes were needed. - Also, fixed "update_stream_privacy_choices" function to not use ":visible" in selectors. --- web/src/settings_components.ts | 6 ++--- web/src/settings_org.ts | 2 +- web/src/stream_edit.ts | 16 ++++++++----- web/src/stream_ui_updates.ts | 24 +++++++------------ web/styles/subscriptions.css | 11 +++++---- .../stream_settings/stream_types.hbs | 4 ++-- 6 files changed, 32 insertions(+), 31 deletions(-) diff --git a/web/src/settings_components.ts b/web/src/settings_components.ts index be08a585c5..ba80101d67 100644 --- a/web/src/settings_components.ts +++ b/web/src/settings_components.ts @@ -1281,7 +1281,7 @@ export function save_discard_stream_settings_widget_status_handler( !sub.subscribed && switching_to_private(properties_elements) ) { - if ($("#stream_permission_settings .stream_privacy_warning").length > 0) { + if ($("#stream_settings .stream_privacy_warning").length > 0) { return; } const context = { @@ -1294,11 +1294,11 @@ export function save_discard_stream_settings_widget_status_handler( classname: "stream_privacy_warning", stream_id: sub.stream_id, }; - $("#stream_permission_settings .stream-permissions-warning-banner").append( + $("#stream_settings .stream-permissions-warning-banner").append( $(render_compose_banner(context)), ); } else { - $("#stream_permission_settings .stream-permissions-warning-banner").empty(); + $("#stream_settings .stream-permissions-warning-banner").empty(); } } diff --git a/web/src/settings_org.ts b/web/src/settings_org.ts index 6dab846db3..5123bbc9f8 100644 --- a/web/src/settings_org.ts +++ b/web/src/settings_org.ts @@ -617,7 +617,7 @@ export function discard_stream_property_element_changes( // Hide stream privacy warning banner const $stream_permissions_warning_banner = $( - "#stream_permission_settings .stream-permissions-warning-banner", + "#stream_settings .stream-permissions-warning-banner", ); if (!$stream_permissions_warning_banner.is(":empty")) { $stream_permissions_warning_banner.empty(); diff --git a/web/src/stream_edit.ts b/web/src/stream_edit.ts index 0310ccf489..925ed88a5a 100644 --- a/web/src/stream_edit.ts +++ b/web/src/stream_edit.ts @@ -586,7 +586,7 @@ export function initialize(): void { ".stream-permissions-warning-banner .main-view-banner-close-button", (event) => { event.preventDefault(); - $("#stream_permission_settings .stream-permissions-warning-banner").empty(); + $("#stream_settings .stream-permissions-warning-banner").empty(); }, ); @@ -608,7 +608,7 @@ export function initialize(): void { const sub = sub_store.get(stream_id); assert(sub !== undefined); stream_settings_components.sub_or_unsub(sub, $stream_row); - $("#stream_permission_settings .stream-permissions-warning-banner").empty(); + $("#stream_settings .stream-permissions-warning-banner").empty(); }, ); @@ -844,8 +844,10 @@ export function initialize(): void { $subsection, sub, ); - if (sub && $subsection.attr("id") === "stream_permission_settings") { - stream_ui_updates.update_default_stream_and_stream_privacy_state($subsection); + if (sub && $subsection.hasClass("stream-permissions")) { + stream_ui_updates.update_default_stream_and_stream_privacy_state( + $("#stream_settings"), + ); const $edit_container = stream_settings_containers.get_edit_container(sub); stream_ui_updates.update_can_subscribe_group_label($edit_container); } @@ -908,8 +910,10 @@ export function initialize(): void { const $subsection = $(this).closest(".settings-subsection-parent"); settings_org.discard_stream_settings_subsection_changes($subsection, sub); - if ($subsection.attr("id") === "stream_permission_settings") { - stream_ui_updates.update_default_stream_and_stream_privacy_state($subsection); + if ($subsection.hasClass("stream-permissions")) { + stream_ui_updates.update_default_stream_and_stream_privacy_state( + $("#stream_settings"), + ); const $edit_container = stream_settings_containers.get_edit_container(sub); stream_ui_updates.update_can_subscribe_group_label($edit_container); } diff --git a/web/src/stream_ui_updates.ts b/web/src/stream_ui_updates.ts index e15ebdb456..d75a24a188 100644 --- a/web/src/stream_ui_updates.ts +++ b/web/src/stream_ui_updates.ts @@ -66,10 +66,10 @@ export function update_web_public_stream_privacy_option_state($container: JQuery )}']`, ); - const for_stream_edit_panel = $container.attr("id") === "stream_permission_settings"; + const for_stream_edit_panel = $container.attr("id") === "stream_settings"; if (for_stream_edit_panel) { const stream_id = Number.parseInt( - $container.closest(".subscription_settings.show").attr("data-stream-id")!, + $container.find(".subscription_settings.show").attr("data-stream-id")!, 10, ); const sub = sub_store.get(stream_id); @@ -334,7 +334,7 @@ export function enable_or_disable_permission_settings_in_edit_panel( const $stream_settings = stream_settings_containers.get_edit_container(sub); - const $general_settings_container = $stream_settings.find($("#stream_permission_settings")); + const $general_settings_container = $stream_settings.find(".stream-permissions"); $general_settings_container .find("input, button") .prop("disabled", !sub.can_change_stream_permissions_requiring_metadata_access); @@ -370,7 +370,7 @@ export function enable_or_disable_permission_settings_in_edit_panel( $advanced_configurations_container, ); - update_default_stream_and_stream_privacy_state($stream_settings); + update_default_stream_and_stream_privacy_state($("#stream_settings")); const disable_message_retention_setting = !realm.zulip_plan_is_not_limited || !current_user.is_owner; @@ -381,10 +381,8 @@ export function enable_or_disable_permission_settings_in_edit_panel( .find(".message-retention-setting-custom-input") .prop("disabled", disable_message_retention_setting); - const $stream_permission_settings = $("#stream_permission_settings"); - - update_web_public_stream_privacy_option_state($stream_permission_settings); - update_public_stream_privacy_option_state($stream_permission_settings); + update_web_public_stream_privacy_option_state($("#stream_settings")); + update_public_stream_privacy_option_state($("#stream_settings")); if (!sub.can_change_stream_permissions_requiring_content_access) { const $stream_privacy_values = $stream_settings @@ -590,17 +588,13 @@ export function update_stream_privacy_choices(policy: string): void { if (!overlays.streams_open()) { return; } - // eslint-disable-next-line no-jquery/no-sizzle - const stream_edit_panel_opened = $("#stream_permission_settings").is(":visible"); - // eslint-disable-next-line no-jquery/no-sizzle - const stream_creation_form_opened = $("#stream-creation").is(":visible"); - if (!stream_edit_panel_opened && !stream_creation_form_opened) { + if ($("#subscription_overlay .nothing-selected").css("display") !== "none") { return; } let $container = $("#stream-creation"); - if (stream_edit_panel_opened) { - $container = $("#stream_permission_settings"); + if ($("#stream_settings").css("display") !== "none") { + $container = $("#stream_settings"); } if (policy === "can_create_private_channel_group") { diff --git a/web/styles/subscriptions.css b/web/styles/subscriptions.css index a9eabce335..c51aad1033 100644 --- a/web/styles/subscriptions.css +++ b/web/styles/subscriptions.css @@ -890,7 +890,7 @@ h4.user_group_setting_subsection_title { .org-permissions-form, .group-permissions, -.stream-permissions { +.advanced-configurations-container { .group_setting_disabled { cursor: not-allowed; /* This ensures that we do not see the not allowed cursor in the @@ -1262,6 +1262,7 @@ div.settings-radio-input-parent { } .stream-permissions, +.advanced-configurations-container, .stream-creation-body, .group-permissions { .input-group { @@ -1306,7 +1307,7 @@ div.settings-radio-input-parent { } .group-permissions .pill-container, -.stream-permissions .pill-container { +.advanced-configurations-container .pill-container { /* Subtract 2 * (2px padding) + 2 * (1px border) */ min-width: calc(var(--modal-input-width) - 6px); background-color: hsl(0deg 0% 100%); @@ -1321,7 +1322,7 @@ div.settings-radio-input-parent { } } -.stream-permissions .pill-container { +.advanced-configurations-container .pill-container { margin-bottom: 10px; } @@ -1345,7 +1346,9 @@ div.settings-radio-input-parent { box-sizing: border-box; } -.stream-permissions .stream-message-retention-days-input input[type="text"] { +.advanced-configurations-container + .stream-message-retention-days-input + input[type="text"] { border-radius: 5px; box-shadow: none; margin: 0; diff --git a/web/templates/stream_settings/stream_types.hbs b/web/templates/stream_settings/stream_types.hbs index cde738fe53..bf42ad63d4 100644 --- a/web/templates/stream_settings/stream_types.hbs +++ b/web/templates/stream_settings/stream_types.hbs @@ -1,4 +1,4 @@ -
+
{{#if is_stream_edit}}

{{t "Channel permissions" }} @@ -43,7 +43,7 @@

-
+