feat(ui): improve modal width rules (#10246)

Followup to https://codeberg.org/forgejo/forgejo/pulls/9636, https://codeberg.org/forgejo/forgejo/pulls/8859#issuecomment-6651595.

1. Due to lack of `min-width`, currently the new consistent dialogs can get disproportionally small to the screen. This PR adds a min-width of 400px. No deep consideration went into choosing this particular width.
    * To make the test not depend on modals we have in the UI with some arbitrary widths a devtest page was added instead
2. Use more horizontal space on narrow screens

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10246
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
0ko
2025-11-28 19:38:50 +01:00
parent f7eb4918d4
commit 79c47c2e50
4 changed files with 111 additions and 4 deletions
+41
View File
@@ -0,0 +1,41 @@
{{template "base/head" .}}
<div class="page-content devtest ui container">
<h1>Modals</h1>
<div class="button-sequence">
<button class="secondary button show-modal" data-modal="#short-modal">Short</button>
<button class="secondary button show-modal" data-modal="#medium-modal">Medium</button>
<button class="secondary button show-modal" data-modal="#long-modal">Long</button>
</div>
<dialog id="short-modal">
<article>
<header>Short modal</header>
<div class="content">🐈</div>
<footer class="actions">
<button class="secondary button cancel">{{ctx.Locale.Tr "settings.cancel"}}</button>
</footer>
</article>
</dialog>
<dialog id="medium-modal">
<article>
<header>Medium modal</header>
<div class="content">aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</div>
<footer class="actions">
<button class="secondary button cancel">{{ctx.Locale.Tr "settings.cancel"}}</button>
</footer>
</article>
</dialog>
<dialog id="long-modal">
<article>
<header>Long modal</header>
<div class="content">aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</div>
<footer class="actions">
<button class="secondary button cancel">{{ctx.Locale.Tr "settings.cancel"}}</button>
</footer>
</article>
</dialog>
</div>
{{template "base/footer" .}}
+1 -1
View File
@@ -107,7 +107,7 @@ func TestE2e(t *testing.T) {
defer test.MockVariableValue(&setting.Quota.Enabled, true)()
defer test.MockVariableValue(&testE2eWebRoutes, routers.NormalRoutes())()
}
if testname == "buttons.test.e2e" || testname == "dropdown.test.e2e" {
if testname == "buttons.test.e2e" || testname == "dropdown.test.e2e" || testname == "modal.test.e2e" {
defer test.MockVariableValue(&setting.IsProd, false)()
defer test.MockVariableValue(&testE2eWebRoutes, routers.NormalRoutes())()
}
+60
View File
@@ -1,4 +1,8 @@
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
// @watch start
// templates/devtest/modal.tmpl
// templates/repo/editor/edit.tmpl
// templates/repo/editor/patch.tmpl
// web_src/js/features/repo-editor.js
@@ -43,3 +47,59 @@ test('Dialog modal', async ({page}) => {
await page.locator('#edit-empty-content-modal .ok').click();
await expect(page).toHaveURL(`/user2/repo1/src/branch/master/${filename}`);
});
test('Dialog modal: width', async ({page, isMobile}) => {
// This test doesn't need JS and runs a little faster without it
await page.goto('/devtest/modal');
// Open modal with short content
const shortModal = page.locator('#short-modal');
await expect(shortModal).toBeHidden();
await page.locator('button[data-modal="#short-modal"]').click();
await expect(shortModal).toBeVisible();
// Check it's width
let width = Math.round((await shortModal.boundingBox()).width);
if (isMobile) {
// Bound by viewport width
expect(width).toBeLessThan(400);
} else {
// Bound by min-width
expect(width).toBe(400);
}
// Open modal with medium sized content
await shortModal.locator('button.cancel').click();
const mediumModal = page.locator('#medium-modal');
await expect(mediumModal).toBeHidden();
await page.locator('button[data-modal="#medium-modal"]').click();
await expect(mediumModal).toBeVisible();
// Check it's width
width = Math.round((await mediumModal.boundingBox()).width);
if (isMobile) {
// Bound by viewport width
expect(width).toBeLessThan(400);
} else {
// Not bound by min-width or max-width
expect(width).toBeLessThan(800);
expect(width).toBeGreaterThan(400);
}
// Open modal with long content
await mediumModal.locator('button.cancel').click();
const longModal = page.locator('#long-modal');
await expect(longModal).toBeHidden();
await page.locator('button[data-modal="#long-modal"]').click();
await expect(longModal).toBeVisible();
// Check it's width
width = Math.round((await longModal.boundingBox()).width);
if (isMobile) {
// Bound by viewport width
expect(width).toBeLessThan(400);
} else {
// Bound by max-width
expect(width).toBe(800);
}
});
+9 -3
View File
@@ -14,13 +14,19 @@ dialog {
1px 3px 15px 2px var(--color-shadow);
border-radius: 0.28571429rem;
outline: none;
padding: 0;
max-width: min(800px, 90vw);
width: fit-content;
z-index: 1001;
pointer-events: auto;
touch-action: auto;
/* Modals shouldn't be wider than 800px ever. On narrow screens they shouldn't
* be wider than the screen, with a small margin */
max-width: min(800px, calc(100vw - var(--page-margin-x) * 2));
/* Modals also should't be tiny compared to the screen width */
min-width: min(400px, calc(100vw - var(--page-margin-x) * 2));
/* Suppress default <dialog> padding */
padding: 0;
}
dialog[open],