From 1e8bfa710ee97c40047be7350f582294a2ea31c4 Mon Sep 17 00:00:00 2001 From: Ganesh Pawar Date: Sun, 4 Jul 2021 12:17:08 +0530 Subject: [PATCH] dialog_widget: Migrate modal to Micromodal. Also removed the `danger_submit_button` config option from the dialog_widget since it isn't needed in the new modals. --- frontend_tests/puppeteer_lib/common.ts | 10 ++ .../puppeteer_tests/delete-message.ts | 12 +- .../puppeteer_tests/realm-linkifier.ts | 8 +- .../puppeteer_tests/user-deactivation.ts | 4 +- package.json | 1 + static/js/click_handlers.js | 1 + static/js/confirm_dialog.js | 1 - static/js/dialog_widget.js | 110 ++++++------ static/js/overlays.js | 80 ++++++++- static/js/settings.js | 2 +- static/styles/modal.css | 167 ++++++++++++++++++ static/styles/night_mode.css | 13 +- static/styles/settings.css | 21 --- static/templates/dialog_widget.hbs | 39 ++-- static/templates/dialog_widget_heading.hbs | 6 - version.py | 2 +- yarn.lock | 5 + 17 files changed, 363 insertions(+), 119 deletions(-) delete mode 100644 static/templates/dialog_widget_heading.hbs diff --git a/frontend_tests/puppeteer_lib/common.ts b/frontend_tests/puppeteer_lib/common.ts index 424160774b..11e25af572 100644 --- a/frontend_tests/puppeteer_lib/common.ts +++ b/frontend_tests/puppeteer_lib/common.ts @@ -496,6 +496,16 @@ class CommonUtils { ); } + async wait_for_micromodal_to_open(page: Page): Promise { + // We manually add the `modal--open` class to the modal after the modal animation completes. + await page.waitForFunction(() => document.querySelector(".modal--open") !== null); + } + + async wait_for_micromodal_to_close(page: Page): Promise { + // This function will ensure that the mouse events are enabled for the background for further tests. + await page.waitForFunction(() => document.querySelector(".modal--open") === null); + } + async run_test_async(test_function: (page: Page) => Promise): Promise { // Pass a page instance to test so we can take // a screenshot of it when the test fails. diff --git a/frontend_tests/puppeteer_tests/delete-message.ts b/frontend_tests/puppeteer_tests/delete-message.ts index d9f42094f9..87a269f045 100644 --- a/frontend_tests/puppeteer_tests/delete-message.ts +++ b/frontend_tests/puppeteer_tests/delete-message.ts @@ -18,13 +18,11 @@ async function delete_message_test(page: Page): Promise { const messages_quantitiy = await page.evaluate(() => $("#zhome .message_row").length); const last_message_id = await click_delete_and_return_last_msg_id(page); - await page.waitForSelector("#dialog_widget_modal", {visible: true}); - await page.click(".dialog_submit_button"); - - const confirm_span = ".dialog_submit_button span"; - await page.waitForSelector(confirm_span, {hidden: true}); - - await page.waitForSelector("#dialog_widget_modal", {hidden: true}); + await common.wait_for_micromodal_to_open(page); + await page.evaluate(() => { + (document.querySelector(".dialog_submit_button") as HTMLButtonElement)?.click(); + }); + await common.wait_for_micromodal_to_close(page); await page.waitForFunction( (expected_length: number) => $("#zhome .message_row").length === expected_length, diff --git a/frontend_tests/puppeteer_tests/realm-linkifier.ts b/frontend_tests/puppeteer_tests/realm-linkifier.ts index 7f9a5fa158..c0383c48a6 100644 --- a/frontend_tests/puppeteer_tests/realm-linkifier.ts +++ b/frontend_tests/puppeteer_tests/realm-linkifier.ts @@ -56,7 +56,7 @@ async function test_add_invalid_linkifier_pattern(page: Page): Promise { async function test_edit_linkifier(page: Page): Promise { await page.click(".linkifier_row .edit"); - await page.waitForFunction(() => document.activeElement?.id === "dialog_widget_modal"); + await common.wait_for_micromodal_to_open(page); await common.fill_form(page, "form.linkifier-edit-form", { pattern: "(?P[0-9a-f]{40})", url_format_string: "https://trac.example.com/commit/%(num)s", @@ -64,7 +64,7 @@ async function test_edit_linkifier(page: Page): Promise { await page.click(".dialog_submit_button"); await page.waitForSelector("#dialog_widget_modal", {hidden: true}); - await common.wait_for_modal_to_close(page); + await common.wait_for_micromodal_to_close(page); await page.waitForSelector(".linkifier_row", {visible: true}); await page.waitForFunction( @@ -81,7 +81,7 @@ async function test_edit_linkifier(page: Page): Promise { async function test_edit_invalid_linkifier(page: Page): Promise { await page.click(".linkifier_row .edit"); - await page.waitForFunction(() => document.activeElement?.id === "dialog_widget_modal"); + await common.wait_for_micromodal_to_open(page); await common.fill_form(page, "form.linkifier-edit-form", { pattern: "#(?Pd????)", url_format_string: "????", @@ -107,7 +107,7 @@ async function test_edit_invalid_linkifier(page: Page): Promise { ); assert.strictEqual(edit_linkifier_format_status, "Failed: Enter a valid URL."); - await page.click(".close-modal-btn"); + await page.click(".dialog_cancel_button"); await page.waitForSelector("#dialog_widget_modal", {hidden: true}); await page.waitForSelector(".linkifier_row", {visible: true}); diff --git a/frontend_tests/puppeteer_tests/user-deactivation.ts b/frontend_tests/puppeteer_tests/user-deactivation.ts index 8121b66fc4..6ecd436153 100644 --- a/frontend_tests/puppeteer_tests/user-deactivation.ts +++ b/frontend_tests/puppeteer_tests/user-deactivation.ts @@ -23,7 +23,7 @@ async function test_deactivate_user(page: Page): Promise { await page.waitForSelector(cordelia_user_row, {visible: true}); await page.waitForSelector(cordelia_user_row + " .fa-user-times"); await page.click(cordelia_user_row + " .deactivate"); - await page.waitForSelector("#dialog_widget_modal", {visible: true}); + await common.wait_for_micromodal_to_open(page); assert.strictEqual( await common.get_text_from_selector(page, ".dialog_heading"), @@ -36,7 +36,7 @@ async function test_deactivate_user(page: Page): Promise { "Deactivate button has incorrect text.", ); await page.click("#dialog_widget_modal .dialog_submit_button"); - await page.waitForSelector("#user-field-status", {hidden: true}); + await common.wait_for_micromodal_to_close(page); } async function test_reactivate_user(page: Page): Promise { diff --git a/package.json b/package.json index c759faaed3..da21f80cda 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "jquery-validation": "^1.19.0", "katex": "^0.13.2", "lodash": "^4.17.19", + "micromodal": "^0.4.6", "mini-css-extract-plugin": "^2.2.2", "plotly.js": "^2.0.0", "postcss": "^8.0.3", diff --git a/static/js/click_handlers.js b/static/js/click_handlers.js index 86794fde71..2da02bc985 100644 --- a/static/js/click_handlers.js +++ b/static/js/click_handlers.js @@ -872,6 +872,7 @@ export function initialize() { !$(e.target).closest(".overlay").length && !$(e.target).closest(".popover").length && !$(e.target).closest(".modal").length && + !$(e.target).closest(".micromodal").length && !$(e.target).closest("[data-tippy-root]").length && !$(e.target).closest(".modal-backdrop").length && $(e.target).closest("body").length diff --git a/static/js/confirm_dialog.js b/static/js/confirm_dialog.js index 36b0beeef4..5d7e7ba294 100644 --- a/static/js/confirm_dialog.js +++ b/static/js/confirm_dialog.js @@ -5,7 +5,6 @@ export function launch(conf) { dialog_widget.launch({ ...conf, close_on_submit: true, - danger_submit_button: true, focus_submit_on_open: true, html_submit_button: $t_html({defaultMessage: "Confirm"}), // Used to control button colors in the template. diff --git a/static/js/dialog_widget.js b/static/js/dialog_widget.js index 013e271b08..36c23d0caa 100644 --- a/static/js/dialog_widget.js +++ b/static/js/dialog_widget.js @@ -1,62 +1,66 @@ import $ from "jquery"; +import Micromodal from "micromodal"; import render_dialog_widget from "../templates/dialog_widget.hbs"; -import render_dialog_heading from "../templates/dialog_widget_heading.hbs"; import * as blueslip from "./blueslip"; import {$t_html} from "./i18n"; import * as loading from "./loading"; import * as overlays from "./overlays"; -import * as settings_data from "./settings_data"; /* - Look for dialog_widget in settings_users - to see an example of how to use this widget. It's - pretty simple to use! - - Some things to note: - - 1) We create DOM on the fly, and we remove - the DOM once it's closed. - - 2) We attach the DOM for the modal to the body element - to avoid style interference from other elements. - - 3) The cancel button is driven by bootstrap.js. - - 4) For settings, we have a click handler in settings.js - that will close the dialog via overlays.close_active_modal. - - 5) We assume that since this is a modal, you will - only ever have one dialog active at any - time. - - 6) If a modal wants a loading spinner, it should pass loading_spinner: true. - This will show a loading spinner when the yes button is clicked. - The caller is responsible for calling hide_dialog_spinner() - to hide the spinner in both success and error handlers. - - 7) If a caller needs to run code after the modal body is added - to DOM, it can do so by passing a post_render hook. -*/ + * Look for confirm_dialog in settings_user_groups + * to see an example of how to use this widget. It's + * pretty simple to use! + * + * Some things to note: + * 1) We create DOM on the fly, and we remove + * the DOM once it's closed. + * + * 2) We attach the DOM for the modal to the body element + * to avoid interference from other elements. + * + * 3) For settings, we have a click handler in settings.js + * that will close the dialog via overlays.close_active_modal. + * + * 4) We assume that since this is a modal, you will + * only ever have one confirm dialog active at any + * time. + * + * 5) If a modal wants a loading spinner, it should pass loading_spinner: true. + * This will show a loading spinner when the yes button is clicked. + * The caller is responsible for calling hide_confirm_dialog_spinner() + * to hide the spinner in both success and error handlers. + * + * 6) If loading_spinner is used, don't hide it on `success`. This modal has a fade out + * animation. This causes the `Confirm` button to be shown for a split second if the + * spinner is hidden. + * Just close the modal. This will remove the whole modal from the DOM without + * needing to remove the spinner. + * + * 7) If a caller needs to run code after the modal body is added + * to DOM, it can do so by passing a post_render hook. + */ export function hide_dialog_spinner() { - $(".dialog_submit_button .loader").hide(); $(".dialog_submit_button span").show(); - $(".dialog_submit_button").prop("disabled", false); - $("#dialog_widget_modal .close-modal-btn").prop("disabled", false); + $("#dialog_widget_modal .modal__btn").prop("disabled", false); + + const spinner = $("#dialog_widget_modal .modal__spinner"); + loading.destroy_indicator(spinner); } export function show_dialog_spinner() { - const using_dark_theme = settings_data.using_dark_theme(); - loading.show_button_spinner($(".dialog_submit_button .loader"), using_dark_theme); $(".dialog_submit_button span").hide(); - $(".dialog_submit_button").prop("disabled", true); - $("#dialog_widget_modal .close-modal-btn").prop("disabled", true); + // Disable both the buttons. + $("#dialog_widget_modal .modal__btn").prop("disabled", true); + + const spinner = $("#dialog_widget_modal .modal__spinner"); + loading.make_indicator(spinner); } export function close_modal() { - overlays.close_modal("#dialog_widget_modal"); + Micromodal.close("dialog_widget_modal"); } export function launch(conf) { @@ -73,7 +77,6 @@ export function launch(conf) { // * html_submit_button: Submit button text. // * close_on_submit: Whether to close modal on clicking submit. // * focus_submit_on_open: Whether to focus submit button on open. - // * danger_submit_button: Whether to use danger button styling for submit button. // * help_link: A help link in the heading area. for (const f of mandatory_fields) { @@ -89,15 +92,11 @@ export function launch(conf) { } const html_submit_button = conf.html_submit_button || $t_html({defaultMessage: "Save changes"}); - const html_dialog_heading = render_dialog_heading({ + const html = render_dialog_widget({ heading_text: conf.html_heading, link: conf.help_link, - }); - const html = render_dialog_widget({ html_submit_button, - html_dialog_heading, html_body: conf.html_body, - danger_submit_button: conf.danger_submit_button, }); const dialog = $(html); $("body").append(dialog); @@ -118,16 +117,15 @@ export function launch(conf) { conf.on_click(e); }); - dialog.on("hidden.bs.modal", () => { - dialog.remove(); + overlays.open_modal("dialog_widget_modal", { + autoremove: true, + micromodal: true, + micromodal_opts: { + onShow: () => { + if (conf.focus_submit_on_open) { + submit_button.trigger("focus"); + } + }, + }, }); - - if (conf.focus_submit_on_open) { - dialog.on("shown.bs.modal", () => { - submit_button.trigger("focus"); - }); - } - - // Open the modal - overlays.open_modal("#dialog_widget_modal"); } diff --git a/static/js/overlays.js b/static/js/overlays.js index 0b91fc1f56..7475a489fd 100644 --- a/static/js/overlays.js +++ b/static/js/overlays.js @@ -1,4 +1,5 @@ import $ from "jquery"; +import Micromodal from "micromodal"; import * as blueslip from "./blueslip"; import * as browser_history from "./browser_history"; @@ -19,7 +20,8 @@ export function is_active() { } export function is_modal_open() { - return $(".modal").hasClass("in"); + // Check for both Bootstrap and Micromodal modals. + return $(".modal").hasClass("in") || $(".micromodal").hasClass("modal--open"); } export function info_overlay_open() { @@ -65,6 +67,12 @@ export function active_modal() { blueslip.error("Programming error — Called active_modal when there is no modal open"); return undefined; } + + // Check for Micromodal modals. + const micromodal = $(".micromodal.modal--open"); + if (micromodal.length) { + return `#${CSS.escape(micromodal.attr("id"))}`; + } return `#${CSS.escape($(".modal.in").attr("id"))}`; } @@ -113,17 +121,25 @@ export function open_overlay(opts) { // If conf.autoremove is true, the modal element will be removed from the DOM // once the modal is hidden. +// If conf.micromodal is true, open a micromodal modal else open a bootstrap modal export function open_modal(selector, conf) { if (selector === undefined) { blueslip.error("Undefined selector was passed into open_modal"); return; } - if (selector[0] !== "#") { + if ((!conf || (conf && !conf.micromodal)) && selector[0] !== "#") { blueslip.error("Non-id-based selector passed in to open_modal: " + selector); return; } + // Don't accept hash-based selector to enforce modals to have unique ids and + // since micromodal doesn't accept hash based selectors. + if (conf && conf.micromodal && selector[0] === "#") { + blueslip.error("hash-based selector passed in to micromodal-based open_modal: " + selector); + return; + } + if (is_modal_open()) { blueslip.error("open_modal() was called while " + active_modal() + " modal was open."); return; @@ -131,6 +147,46 @@ export function open_modal(selector, conf) { blueslip.debug("open modal: " + selector); + // Show a modal using micromodal. + if (conf && conf.micromodal) { + // Micromodal gets elements using the getElementById DOM function + // which doesn't require the hash. We add it manually here. + const id_selector = `#${selector}`; + const micromodal = $(id_selector); + + micromodal.find(".modal__container").on("animationend", (event) => { + // Micromodal doesn't support Bootstrap-style `shown.bs.modal` and + // `hidden.bs.modal` events. We workaround this by using the animationName + // from the native event and running the required functions after the + // animation ends. + const animation_name = event.originalEvent.animationName; + if (animation_name === "mmfadeIn") { + // Equivalent to bootstrap's "shown.bs.modal" event + + // Micromodal adds the is-open class before the modal animation + // is complete, which isn't really helpful since a modal is open after the + // animation is complete. So, we manually add a class after the + // animation is complete. + micromodal.addClass("modal--open"); + micromodal.removeClass("modal--opening"); + } else if (animation_name === "mmfadeOut") { + // Equivalent to bootstrap's "hidden.bs.modal" event + + micromodal.removeClass("modal--open"); + if (conf.autoremove) { + micromodal.remove(); + } + } + }); + + Micromodal.show(selector, { + disableFocus: true, + openClass: "modal--opening", + ...conf.micromodal_opts, + }); + return; + } + const elem = $(selector).expectOne(); elem.modal("show").attr("aria-hidden", false); // Disable background mouse events when modal is active @@ -185,7 +241,8 @@ export function close_active() { close_overlay(open_overlay_name); } -export function close_modal(selector) { +// If conf.micromodal is true, close a micromodal modal else close a bootstrap modal +export function close_modal(selector, conf) { if (selector === undefined) { blueslip.error("Undefined selector was passed into close_modal"); return; @@ -196,7 +253,10 @@ export function close_modal(selector) { return; } - if (active_modal() !== selector) { + if ( + (!conf && active_modal() !== selector) || + (conf && conf.micromodal && active_modal() !== `#${selector}`) + ) { blueslip.error( "Trying to close " + selector + " modal when " + active_modal() + " is open.", ); @@ -205,6 +265,11 @@ export function close_modal(selector) { blueslip.debug("close modal: " + selector); + if (conf && conf.micromodal) { + Micromodal.close(selector); + return; + } + const elem = $(selector).expectOne(); elem.modal("hide").attr("aria-hidden", true); } @@ -215,6 +280,13 @@ export function close_active_modal() { return; } + // Check for Micromodal modals. + const micromodal = $(".micromodal.modal--open"); + if (micromodal.length) { + Micromodal.close(`${CSS.escape(micromodal.attr("id"))}`); + return; + } + $(".modal.in").modal("hide").attr("aria-hidden", true); } diff --git a/static/js/settings.js b/static/js/settings.js index 2712bc2305..977dc3bdec 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -28,7 +28,7 @@ $("body").ready(() => { if (!overlays.is_modal_open()) { return; } - if ($(e.target).closest(".modal").length > 0) { + if ($(e.target).closest(".modal, .micromodal").length > 0) { return; } e.preventDefault(); diff --git a/static/styles/modal.css b/static/styles/modal.css index 56fbf61123..aa7c58bde8 100644 --- a/static/styles/modal.css +++ b/static/styles/modal.css @@ -35,3 +35,170 @@ .modal-bg { background-color: hsl(0, 0%, 98%); } + +/* Styles for the Micromodal-based modals */ +.modal__overlay { + position: fixed; + top: 0; + left: 0; + right: 0; + bottom: 0; + background: hsla(0, 0%, 0%, 0.6); + display: flex; + justify-content: center; + align-items: center; + z-index: 105; +} + +.modal__container { + display: flex; + flex-direction: column; + background-color: hsl(0, 0%, 100%); + max-width: calc(100% - 32px); + max-height: 96%; + width: 32.5rem; + border-radius: 4px; + box-sizing: border-box; +} + +.modal__header { + padding: 16px 24px; + display: flex; + justify-content: space-between; + align-items: center; +} + +.modal__footer { + display: flex; + justify-content: flex-end; + align-items: center; + padding: 20px 24px; +} + +.modal__title { + margin: 0; + font-size: 1.375rem; + line-height: 1.25; +} + +.modal__close { + &::before { + content: "\2715"; + } + margin-right: -4px; + background: transparent; + border: 0; + + &:hover { + background: hsl(0, 0%, 90%); + } +} + +.modal__content { + font-size: 1rem; + overflow-y: auto; + padding: 0 24px; + line-height: 1.5; +} + +.modal__btn { + font-size: 0.875rem; + padding: 0.5rem 1rem; + background-color: hsl(0, 0%, 90%); + border-radius: 0.25rem; + border-width: 0; + cursor: pointer; + appearance: button; + text-transform: none; + overflow: visible; + outline: none !important; + line-height: 1.15; + margin: 0; + will-change: transform; + backface-visibility: hidden; + transform: translateZ(0); + transition: transform 0.25s ease-out; + + &:focus { + box-shadow: hsl(198, 76%, 47%) 0 0 0 1px, + hsla(198, 76%, 47%, 0.3) 0 0 0 5px; + } +} + +.modal__btn:focus, +.modal__btn:hover { + transform: scale(1.05); +} + +.dialog_cancel_button { + background: hsl(0, 0%, 100%); + border: 1px solid hsla(300, 2%, 11%, 0.3); + + &:hover { + background: hsl(0, 0%, 97%); + } +} + +.dialog_submit_button { + margin-left: 12px; + background-color: hsl(214, 100%, 31%); + color: hsl(0, 0%, 100%); +} + +@keyframes mmfadeIn { + from { + opacity: 0; + } + + to { + opacity: 1; + } +} + +@keyframes mmfadeOut { + from { + opacity: 1; + } + + to { + opacity: 0; + } +} + +.micromodal { + display: none; +} + +.micromodal.modal--opening, +.micromodal.modal--open { + display: block; +} + +.micromodal[aria-hidden="true"] .modal__overlay { + animation: mmfadeOut 75ms cubic-bezier(0, 0, 0.2, 1); +} + +.micromodal[aria-hidden="false"] .modal__overlay { + animation: mmfadeIn 120ms cubic-bezier(0, 0, 0.2, 1); +} + +.micromodal[aria-hidden="true"] .modal__container { + animation: mmfadeOut 75ms cubic-bezier(0, 0, 0.2, 1); +} + +.micromodal[aria-hidden="false"] .modal__container { + animation: mmfadeIn 120ms cubic-bezier(0, 0, 0.2, 1); +} + +.micromodal .modal__container, +.micromodal .modal__overlay { + will-change: transform; +} + +.modal__spinner .loading_indicator_spinner { + height: 16px; + + path { + fill: hsl(0, 0%, 100%); + } +} diff --git a/static/styles/night_mode.css b/static/styles/night_mode.css index 4537eb68d0..25a8299bf8 100644 --- a/static/styles/night_mode.css +++ b/static/styles/night_mode.css @@ -128,10 +128,21 @@ body.night-mode { border-color: hsla(0, 0%, 100%, 0.4); } - .modal-bg { + .modal-bg, + .modal__container { background-color: hsl(212, 28%, 18%); } + .modal__close { + &::before { + color: hsl(236, 33%, 90%); + } + + &:hover { + background: hsla(0, 0%, 91%, 0.1); + } + } + .streams_popover .sp-container { background-color: transparent; diff --git a/static/styles/settings.css b/static/styles/settings.css index 97fab3a0af..fdf482a227 100644 --- a/static/styles/settings.css +++ b/static/styles/settings.css @@ -1577,17 +1577,6 @@ input[type="checkbox"] { } } -/* Dialog widgets should be centered, which this roughly achieves. */ -#dialog_widget_modal { - top: calc(50% - 120px); -} - -/* In the settings overlay, we need slightly different CSS for alignment. */ -#settings_overlay_container #dialog_widget_modal { - top: 50%; - vertical-align: center; -} - /* These have enough space for all the options in German. */ .setting_desktop_icon_count_display, #id_realm_waiting_period_setting, @@ -1618,16 +1607,6 @@ input[type="checkbox"] { margin-top: 10px; } -.dialog_submit_button .loader { - display: none; - vertical-align: top; - position: relative; - height: 30px; - margin-top: -10px; - top: 5px; - width: 30px; -} - .dropdown-list-widget { button { margin: 0 5px; diff --git a/static/templates/dialog_widget.hbs b/static/templates/dialog_widget.hbs index 48af1df59d..ce403431e8 100644 --- a/static/templates/dialog_widget.hbs +++ b/static/templates/dialog_widget.hbs @@ -1,17 +1,26 @@ -