From f9628f883df485af1c208345d55a956db741eb76 Mon Sep 17 00:00:00 2001 From: Beowulf Date: Thu, 25 Apr 2024 22:19:24 +0200 Subject: [PATCH] Move settings button back to the right in repo and org header This will move the settings button back to the right, like known from older versions. For this, the overflow-menu was changed when a setting button is available. If no settings button is available, the behavior will not change. Fixes #3301 --- templates/org/menu.tmpl | 2 +- templates/repo/header.tmpl | 6 +- tests/e2e/right-settings-button.test.e2e.js | 114 ++++++++++++++++++++ web_src/js/webcomponents/overflow-menu.js | 24 ++++- 4 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/right-settings-button.test.e2e.js diff --git a/templates/org/menu.tmpl b/templates/org/menu.tmpl index c519606d1..1860a3765 100644 --- a/templates/org/menu.tmpl +++ b/templates/org/menu.tmpl @@ -40,7 +40,7 @@ {{end}} {{if .IsOrganizationOwner}} - + {{svg "octicon-tools"}} {{ctx.Locale.Tr "repo.settings"}} {{end}} diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index bbc3b9c00..95c2e059d 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -177,18 +177,18 @@ {{$highlightSettings := true}} {{if and .SignedUser.EnableRepoUnitHints (not (.Repository.AllUnitsEnabled ctx))}} {{$highlightSettings = false}} - + {{svg "octicon-diff-added"}} {{ctx.Locale.Tr "repo.settings.units.add_more"}} {{end}} - + {{svg "octicon-tools"}} {{ctx.Locale.Tr "repo.settings"}} {{end}} {{else if .Permission.IsAdmin}}
- + {{svg "octicon-tools"}} {{ctx.Locale.Tr "repo.settings"}}
diff --git a/tests/e2e/right-settings-button.test.e2e.js b/tests/e2e/right-settings-button.test.e2e.js new file mode 100644 index 000000000..7f457dec4 --- /dev/null +++ b/tests/e2e/right-settings-button.test.e2e.js @@ -0,0 +1,114 @@ +// @ts-check +import {test, expect} from '@playwright/test'; +import {login_user, load_logged_in_context} from './utils_e2e.js'; + +test.beforeAll(async ({browser}, workerInfo) => { + await login_user(browser, workerInfo, 'user2'); +}); + +test.describe('desktop viewport', () => { + test.use({viewport: {width: 1920, height: 300}}); + + test('Settings button on right of repo header', async ({browser}, workerInfo) => { + const context = await load_logged_in_context(browser, workerInfo, 'user2'); + const page = await context.newPage(); + + await page.goto('/user2/repo1'); + + const settingsBtn = page.locator('.overflow-menu-items>#settings-btn'); + await expect(settingsBtn).toBeVisible(); + await expect(settingsBtn).toHaveClass(/right/); + + await expect(page.locator('.overflow-menu-button')).toHaveCount(0); + }); + + test('Settings button on right of org header', async ({browser}, workerInfo) => { + const context = await load_logged_in_context(browser, workerInfo, 'user2'); + const page = await context.newPage(); + + await page.goto('/org3'); + + const settingsBtn = page.locator('.overflow-menu-items>#settings-btn'); + await expect(settingsBtn).toBeVisible(); + await expect(settingsBtn).toHaveClass(/right/); + + await expect(page.locator('.overflow-menu-button')).toHaveCount(0); + }); + + test('User overview overflow menu should not be influenced', async ({page}) => { + await page.goto('/user2'); + + await expect(page.locator('.overflow-menu-items>#settings-btn')).toHaveCount(0); + + await expect(page.locator('.overflow-menu-button')).toHaveCount(0); + }); +}); + +test.describe('small viewport', () => { + test.use({viewport: {width: 800, height: 300}}); + + test('Settings button in overflow menu of repo header', async ({browser}, workerInfo) => { + const context = await load_logged_in_context(browser, workerInfo, 'user2'); + const page = await context.newPage(); + + await page.goto('/user2/repo1'); + + await expect(page.locator('.overflow-menu-items>#settings-btn')).toHaveCount(0); + + await expect(page.locator('.overflow-menu-button')).toBeVisible(); + + await page.click('.overflow-menu-button'); + await expect(page.locator('.tippy-target>#settings-btn')).toBeVisible(); + + // Verify that we have no duplicated items + const shownItems = await page.locator('.overflow-menu-items>a').all(); + expect(shownItems).not.toHaveLength(0); + const overflowItems = await page.locator('.tippy-target>a').all(); + expect(overflowItems).not.toHaveLength(0); + + const items = shownItems.concat(overflowItems); + expect(Array.from(new Set(items))).toHaveLength(items.length); + }); + + test('Settings button in overflow menu of org header', async ({browser}, workerInfo) => { + const context = await load_logged_in_context(browser, workerInfo, 'user2'); + const page = await context.newPage(); + + await page.goto('/org3'); + + await expect(page.locator('.overflow-menu-items>#settings-btn')).toHaveCount(0); + + await expect(page.locator('.overflow-menu-button')).toBeVisible(); + + await page.click('.overflow-menu-button'); + await expect(page.locator('.tippy-target>#settings-btn')).toBeVisible(); + + // Verify that we have no duplicated items + const shownItems = await page.locator('.overflow-menu-items>a').all(); + expect(shownItems).not.toHaveLength(0); + const overflowItems = await page.locator('.tippy-target>a').all(); + expect(overflowItems).not.toHaveLength(0); + + const items = shownItems.concat(overflowItems); + expect(Array.from(new Set(items))).toHaveLength(items.length); + }); + + test('User overview overflow menu should not be influenced', async ({page}) => { + await page.goto('/user2'); + + await expect(page.locator('.overflow-menu-items>#settings-btn')).toHaveCount(0); + + await expect(page.locator('.overflow-menu-button')).toBeVisible(); + await page.click('.overflow-menu-button'); + await expect(page.locator('.tippy-target>#settings-btn')).toHaveCount(0); + + // Verify that we have no duplicated items + const shownItems = await page.locator('.overflow-menu-items>a').all(); + expect(shownItems).not.toHaveLength(0); + const overflowItems = await page.locator('.tippy-target>a').all(); + expect(overflowItems).not.toHaveLength(0); + + const items = shownItems.concat(overflowItems); + expect(Array.from(new Set(items))).toHaveLength(items.length); + }); +}); diff --git a/web_src/js/webcomponents/overflow-menu.js b/web_src/js/webcomponents/overflow-menu.js index 604fce7d4..a69ce1681 100644 --- a/web_src/js/webcomponents/overflow-menu.js +++ b/web_src/js/webcomponents/overflow-menu.js @@ -69,13 +69,35 @@ window.customElements.define('overflow-menu', class extends HTMLElement { this.tippyItems = []; const menuRight = this.offsetLeft + this.offsetWidth; const menuItems = this.menuItemsEl.querySelectorAll('.item'); + const settingItem = this.menuItemsEl.querySelector('#settings-btn'); for (const item of menuItems) { const itemRight = item.offsetLeft + item.offsetWidth; - if (menuRight - itemRight < 38) { // roughly the width of .overflow-menu-button + // Width of the settings button plus a small value to get the next item to the left if there is directly one + // If no setting button is in the menu the default threshold is 38 - roughly the width of .overflow-menu-button + const overflowBtnThreshold = 38; + const threshold = settingItem?.offsetWidth ?? overflowBtnThreshold; + // If we have a settings item on the right-hand side, we must also check if the first, + // possibly overflowing item would still fit on the left-hand side of the overflow menu + // If not, it must be added to the array (twice). The duplicate is removed with the shift. + if (settingItem && !this.tippyItems?.length && item !== settingItem && menuRight - itemRight < overflowBtnThreshold) { + this.tippyItems.push(settingItem); + } + if (menuRight - itemRight < threshold) { this.tippyItems.push(item); } } + // Special handling for settings button on right. Only done if a setting item is present + if (settingItem) { + // If less than 2 items overflow, remove all items (only settings "overflowed" - because it's on the right side) + if (this.tippyItems?.length < 2) { + this.tippyItems = []; + } else { + // Remove the first item of the list, because we have always one item more in the array due to the big threshold above + this.tippyItems.shift(); + } + } + // if there are no overflown items, remove any previously created button if (!this.tippyItems?.length) { const btn = this.querySelector('.overflow-menu-button');