Skip to content

Commit

Permalink
[Mobile] Center Confirmation Dialog Texts (#7492)
Browse files Browse the repository at this point in the history
* Make overlay messages centered

* Fix changes so that only dialogs and not forms are affected

* Fix buttons such that they are right-aligned

* Reduce to one worker for stability

* Add test to cover new capabilities

* lint fixes

* Closes #7343
- Fixed an oversight that caused the top of form dialogs to
be scrolled out of view by default.
- Fixed approach to vertical centering for `-fit` type confirmation dialogs.
- Reduced size of confirmation dialog icons.
- Smoke tested in Chrome mobile emulator in a large variety of mobile
viewport sizes and orientations.

* Closes #7343
- Removes extra margin unintentionally added to `l-overlay-large`.

---------

Co-authored-by: John Hill <[email protected]>
Co-authored-by: Charles Hacskaylo <[email protected]>
  • Loading branch information
3 people authored Mar 3, 2024
1 parent e449fd0 commit 73eead6
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 29 deletions.
4 changes: 2 additions & 2 deletions e2e/playwright-mobile.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import { devices } from '@playwright/test';
const MAX_FAILURES = 5;
const NUM_WORKERS = 2;

import { fileURLToPath } from 'url';

Expand All @@ -20,7 +19,8 @@ const config = {
reuseExistingServer: true //This was originally disabled to prevent differences in local debugging vs. CI. However, it significantly speeds up local debugging.
},
maxFailures: MAX_FAILURES, //Limits failures to 5 to reduce CI Waste
workers: NUM_WORKERS, //Limit to 2 for CircleCI Agent
workers: 1, //Limit to 1 due to resource constraints similar to https://github.com/percy/cli/discussions/1067

use: {
baseURL: 'http://localhost:8080/',
headless: true,
Expand Down
24 changes: 20 additions & 4 deletions e2e/tests/mobile/smoke.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,16 @@ Make no assumptions about the order that elements appear in the DOM.
*/

import { expect, test } from '../../pluginFixtures.js';

test('Verify that My Items Tree appears @mobile', async ({ page, openmctConfig }) => {
const { myItemsFolderName } = openmctConfig;
//Go to baseURL
await page.goto('./');

//My Items to be visible
await expect(page.getByRole('treeitem', { name: `${myItemsFolderName}` })).toBeVisible();
});

test('Verify that user can search @mobile', async ({ page }) => {
//For now, this test is going to be hardcoded against './test-data/display_layout_with_child_layouts.json'
await page.goto('./');

await page.getByRole('searchbox', { name: 'Search Input' }).click();
await page.getByRole('searchbox', { name: 'Search Input' }).fill('Parent Display Layout');
//Search Results appear in search modal
Expand All @@ -57,3 +53,23 @@ test('Verify that user can search @mobile', async ({ page }) => {
await page.getByTitle('Collapse Browse Pane').click();
await expect(page.getByRole('main').getByText('Parent Display Layout')).toBeVisible();
});
test('Remove Object and confirmation dialog @mobile', async ({ page }) => {
await page.goto('./');
await page.getByRole('searchbox', { name: 'Search Input' }).click();
await page.getByRole('searchbox', { name: 'Search Input' }).fill('Parent Display Layout');
//Search Results appear in search modal
//Clicking on the search result takes you to the object
await page.getByLabel('Object Results').getByText('Parent Display Layout').click();
await page.getByTitle('Collapse Browse Pane').click();
await expect(page.getByRole('main').getByText('Parent Display Layout')).toBeVisible();
//Verify both objects are in view
await expect(await page.getByLabel('Child Layout 1 Layout')).toBeVisible();
await expect(await page.getByLabel('Child Layout 2 Layout')).toBeVisible();
//Remove First Object to bring up confirmation dialog
await page.getByLabel('View menu items').nth(1).click();
await page.getByLabel('Remove').click();
await page.getByRole('button', { name: 'OK' }).click();
//Verify that the object is removed
await expect(await page.getByLabel('Child Layout 1 Layout')).toBeVisible();
expect(await page.getByLabel('Child Layout 2 Layout').count()).toBe(0);
});
42 changes: 22 additions & 20 deletions src/api/overlays/components/OverlayComponent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,29 @@
class="c-click-icon c-overlay__close-button icon-x"
@click.stop="destroy"
></button>
<div
ref="element"
class="c-overlay__contents js-notebook-snapshot-item-wrapper"
tabindex="0"
aria-modal="true"
aria-label="Overlay"
role="dialog"
></div>
<div v-if="buttons" class="c-overlay__button-bar">
<button
v-for="(button, index) in buttons"
ref="buttons"
:key="index"
class="c-button js-overlay__button"
<div class="c-overlay__content-wrapper">
<div
ref="element"
class="c-overlay__contents js-notebook-snapshot-item-wrapper"
tabindex="0"
:class="{ 'c-button--major': focusIndex === index }"
@focus="focusIndex = index"
@click="buttonClickHandler(button.callback)"
>
{{ button.label }}
</button>
aria-modal="true"
aria-label="Overlay"
role="dialog"
></div>
<div v-if="buttons" class="c-overlay__button-bar">
<button
v-for="(button, index) in buttons"
ref="buttons"
:key="index"
class="c-button js-overlay__button"
tabindex="0"
:class="{ 'c-button--major': focusIndex === index }"
@focus="focusIndex = index"
@click="buttonClickHandler(button.callback)"
>
{{ button.label }}
</button>
</div>
</div>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/api/overlays/components/dialog-component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

&__icon {
// Holds a background SVG graphic
$s: 80px;
$s: 50px;
flex: 0 0 auto;
min-width: $s;
min-height: $s;
Expand Down
36 changes: 34 additions & 2 deletions src/api/overlays/components/overlay-component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,23 @@
z-index: 70;

&__blocker {
display: none; // Mobile-first
// Mobile-first: use the blocker to create a full look to dialogs
@include abs();
background: $colorBodyBg;
}

&__outer {
@include abs();
background: $colorBodyBg;
display: flex;
flex-direction: column;
padding: $overlayInnerMargin;

body.mobile .l-overlay-fit & {
// Vertically center small dialogs in mobile
top: 50%;
bottom: auto;
transform: translateY(-50%);
}
}

&__close-button {
Expand All @@ -39,12 +47,32 @@
z-index: 99;
}

&__content-wrapper {
display: flex;
height: 100%;
overflow: auto;
flex-direction: column;
gap: $interiorMargin;

body.desktop & {
overflow: hidden;
}

.l-overlay-fit &,
.l-overlay-dialog & {
margin: $overlayInnerMargin;
}
}

&__contents {
flex: 1 1 auto;
display: flex;
flex-direction: column;
outline: none;
overflow: auto;
body.mobile & {
flex: none;
}
}

&__top-bar {
Expand Down Expand Up @@ -78,6 +106,10 @@
display: flex;
justify-content: flex-end;
margin-top: $interiorMargin;
body.mobile & {
justify-content: flex-end;
padding-right: $interiorMargin;
}

> * + * {
margin-left: $interiorMargin;
Expand Down

0 comments on commit 73eead6

Please sign in to comment.