Skip to content

[Flexible Layouts] Flexible Layout styling fixes #7319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
39f30c0
Add css class to flexible layouts to allow them to be styled, reset s…
khalidadil Nov 9, 2023
30b34a2
Add tests
khalidadil Dec 20, 2023
ad290cb
Merge branch 'master' into bugfix/issue-6693
khalidadil Dec 20, 2023
5889376
Add tests
khalidadil Dec 20, 2023
6ab76ba
Remove accidental commit
khalidadil Dec 20, 2023
e28abcb
first pass
unlikelyzero Dec 22, 2023
b462c6d
drive-by to improve flex layout git blame
unlikelyzero Dec 22, 2023
3aaf74b
drive-by to improve watch mode
unlikelyzero Dec 22, 2023
aef2432
fix flexlayout toolbar options
unlikelyzero Dec 22, 2023
8bd5757
fix roles for flexlayout to be a grid instead of group
unlikelyzero Dec 22, 2023
3b56b67
driveby git blame ignore
unlikelyzero Dec 23, 2023
8190388
Some more testability improvements
unlikelyzero Dec 26, 2023
8165117
Improving the tests
unlikelyzero Dec 26, 2023
742e8e0
fixes
unlikelyzero Dec 26, 2023
c4a7292
Merge branch 'master' into bugfix/issue-6693
ozyx Dec 27, 2023
5f5c07e
test(e2e): fix styling tests
ozyx Dec 28, 2023
08ed4be
Merge branch 'master' into bugfix/issue-6693
khalidadil Dec 28, 2023
70ad074
test(e2e): fix flex layout toolbar test
ozyx Dec 28, 2023
a8ac603
test(e2e): fix row/column layout test
ozyx Dec 28, 2023
877b54a
a11y: "More options" -> "More actions"
ozyx Dec 29, 2023
3e16f0f
Change name of computed property for options label from getLabel to o…
khalidadil Dec 30, 2023
9945814
split up tests and work on stackedPlotStyling
unlikelyzero Jan 2, 2024
de1f511
Merge branch 'bugfix/issue-6693' of https://github.com/nasa/openmct i…
unlikelyzero Jan 2, 2024
3644534
add border styling
unlikelyzero Jan 2, 2024
72320b2
break out conditional and style inspector
unlikelyzero Jan 2, 2024
5001f66
update flexlayout tests and add stackedplot (wip)
unlikelyzero Jan 2, 2024
2aa76b0
Merge branch 'master' of https://github.com/nasa/openmct into bugfix/…
unlikelyzero Jan 2, 2024
d0fadd3
fix imports
unlikelyzero Jan 3, 2024
ffdba24
Add visual test
unlikelyzero Jan 3, 2024
46d4a29
update stackedplot test
unlikelyzero Jan 3, 2024
8eb5856
Add a11y label
unlikelyzero Jan 3, 2024
f3409b3
add font styles checks
unlikelyzero Jan 3, 2024
5371591
add font checking
unlikelyzero Jan 3, 2024
1b7f0b3
add visual tests
unlikelyzero Jan 3, 2024
cf2681e
Update toolbar to use aria
unlikelyzero Jan 3, 2024
4971bce
linting and comments
unlikelyzero Jan 3, 2024
1df1f72
lint fix
unlikelyzero Jan 3, 2024
6e5fd05
update comments
unlikelyzero Jan 3, 2024
8cf081d
skip test for bug
unlikelyzero Jan 4, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
# Requires Git > 2.23
# See https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt

# vue-eslint update 2019
14a0f84c1bcd56886d7c9e4e6afa8f7d292734e5
# eslint changes 2022
d80b6923541704ab925abf0047cbbc58735c27e2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive-by to make it easier to work with flex layouts

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is so cool

# Copyright year update 2022
4a9744e916d24122a81092f6b7950054048ba860
# Copyright year update 2023
Expand Down
54 changes: 54 additions & 0 deletions e2e/playwright-watch.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/* eslint-disable no-undef */
// playwright.config.js
// @ts-check

// eslint-disable-next-line no-unused-vars
const { devices } = require('@playwright/test');
const MAX_FAILURES = 5;
const NUM_WORKERS = 2;

/** @type {import('@playwright/test').PlaywrightTestConfig} */
const config = {
retries: 0, //Retries 2 times for a total of 3 runs. When running sharded and with max-failures=5, this should ensure that flake is managed without failing the full suite
testDir: 'tests',
timeout: 60 * 1000,
webServer: {
command: 'npm run start', //Start in dev mode for hot reloading
url: 'http://localhost:8080/#',
timeout: 200 * 1000,
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
use: {
baseURL: 'http://localhost:8080/',
headless: true,
ignoreHTTPSErrors: true,
screenshot: 'only-on-failure',
trace: 'on-first-retry',
video: 'off'
},
projects: [
{
name: 'chrome',
testMatch: '**/*.spec.js', // run all tests
use: {
browserName: 'chromium'
}
}
],
reporter: [
['list'],
[
'html',
{
open: 'never',
outputFolder: '../html-test-results' //Must be in different location due to https://github.com/microsoft/playwright/issues/12840
}
],
['junit', { outputFile: '../test-results/results.xml' }],
['@deploysentinel/playwright']
]
};

module.exports = config;
293 changes: 293 additions & 0 deletions e2e/tests/functional/styling.e2e.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
/*****************************************************************************
* Open MCT, Copyright (c) 2014-2023, United States Government
* as represented by the Administrator of the National Aeronautics and Space
* Administration. All rights reserved.
*
* Open MCT is licensed under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* Open MCT includes source code licensed under additional open source
* licenses. See the Open Source Licenses file (LICENSES.md) included with
* this source code distribution or the Licensing information page available
* at runtime from the About dialog for additional information.
*****************************************************************************/

/**
* This test is dedicated to test styling of various plugins
*/

const { test, expect } = require('../../pluginFixtures');
const { createDomainObjectWithDefaults } = require('../../appActions');

test.describe('Flex Layout Style Inspector Options', () => {
let flexibleLayout;
test.beforeEach(async ({ page }) => {
await page.goto('./', { waitUntil: 'domcontentloaded' });

// Create a Flexible Layout and attach to the Stacked Plot
flexibleLayout = await createDomainObjectWithDefaults(page, {
type: 'Flexible Layout',
name: 'Flexible Layout'
});
});

test('selecting a flexible layout column hides the styles tab', async ({ page }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

broke this out to its own suite to improve runtime and improve the opportunity to add new styling specific inspector tests

await page.goto(flexibleLayout.url, { waitUntil: 'domcontentloaded' });

// Edit Flexible Layout
await page.getByLabel('Edit').click();

// Expect to find styles tab
await expect(page.getByRole('tab', { name: 'Styles' })).toBeVisible();

// Select flexible layout column
await page.getByLabel('Container Handle 0').click();

// Expect to find no styles tab
await expect(page.getByRole('tab', { name: 'Styles' })).toBeHidden();
});
});

test.describe('Flexible Layout styling', () => {
let sineWaveObject;
let sineWaveObject2;
let stackedPlot;
let flexibleLayout;
test.beforeEach(async ({ page }) => {
await page.goto('./', { waitUntil: 'domcontentloaded' });

// Create a Flexible Layout and attach to the Stacked Plot
flexibleLayout = await createDomainObjectWithDefaults(page, {
type: 'Flexible Layout',
name: 'Flexible Layout'
});

// Create a Stacked Plot and attach to the Flexible Layout
stackedPlot = await createDomainObjectWithDefaults(page, {
type: 'Stacked Plot',
name: 'Stacked Plot',
parent: flexibleLayout.uuid
});

// Create two SWGs and attach them to the Stacked Plot
sineWaveObject = await createDomainObjectWithDefaults(page, {
type: 'Sine Wave Generator',
name: 'Sine Wave Generator 1',
parent: stackedPlot.uuid
});

sineWaveObject2 = await createDomainObjectWithDefaults(page, {
type: 'Sine Wave Generator',
name: 'Sine Wave Generator 2',
parent: stackedPlot.uuid
});
});

test('styling the flexible layout properly applies the styles to all containers', async ({
page
}) => {
// Values to set styles to
const backgroundColor = '#5b0f00';
const textColor = '#e6b8af';

// Directly navigate to the flexible layout
await page.goto(flexibleLayout.url, { waitUntil: 'domcontentloaded' });

// Edit Flexible Layout
await page.getByLabel('Edit').click();

// Select styles tab
await page.getByRole('tab', { name: 'Styles' }).click();

// Set styles using setStyles function
await setStyles(page, backgroundColor, textColor, page.locator('.c-fl__container-holder'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll need to figure out why the page.locator('.c-fl__container-holder')); locator is the only way to select the holder


// Check styles using checkStyles function
await checkStyles(
page,
hexToRGB(backgroundColor),
hexToRGB(textColor),
page.locator('.c-fl__container-holder')
);

// Save Flexible Layout
await page.getByRole('button', { name: 'Save' }).click();
await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();

// Reload page
await page.reload({ waitUntil: 'domcontentloaded' });

// Check styles using checkStyles function
await checkStyles(
page,
hexToRGB(backgroundColor),
hexToRGB(textColor),
page.locator('.c-fl__container-holder')
);
});

test('styling a child object of the flexible layout properly applies that style to only that child', async ({
page
}) => {
const backgroundColor = '#5b0f00';
const textColor = '#e6b8af';
// Directly navigate to the flexible layout
await page.goto(flexibleLayout.url, { waitUntil: 'domcontentloaded' });

// Edit Flexible Layout
await page.getByLabel('Edit').click();

// Select styles tab
await page.getByRole('tab', { name: 'Styles' }).click();

// Set styles using setStyles function
await setStyles(page, backgroundColor, textColor, page.getByLabel('Stacked Plot Frame'));

// Check styles using checkStyles function
await checkStyles(
page,
hexToRGB(backgroundColor),
hexToRGB(textColor),
page.getByLabel('Stacked Plot Frame')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but not here. I'm thinking is something fundamentally wrong in the way we are selecting elements for comparison.

);

// Save Flexible Layout
await page.getByRole('button', { name: 'Save' }).click();
await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();

// Reload page
await page.reload({ waitUntil: 'domcontentloaded' });

// Check styles using checkStyles function
await checkStyles(
page,
hexToRGB(backgroundColor),
hexToRGB(textColor),
page.getByLabel('Stacked Plot Frame')
);
});

test('When the "no style" option is selected, background and text should be reset to inherited styles', async ({
page
}) => {
// Set background and font color on Stacked Plot object
const backgroundColor = 'rgb(91, 15, 0)';
const textColor = 'rgb(230, 184, 175)';
const inheritedBackgroundColor = '#000000'; // inherited from the theme
const inheritedColor = '#aaaaaa'; // inherited from the body style

// Directly navigate to the flexible layout
await page.goto(flexibleLayout.url, { waitUntil: 'domcontentloaded' });

// Edit Flexible Layout
await page.getByLabel('Edit').click();

// Select styles tab
await page.getByRole('tab', { name: 'Styles' }).click();

// Set styles using setStyles function
await setStyles(page, backgroundColor, textColor, page.getByLabel('Stacked Plot Frame'));

// Check styles using checkStyles function
await checkStyles(
page,
hexToRGB(backgroundColor),
hexToRGB(textColor),
page.getByLabel('Stacked Plot Frame')
);

// Save Flexible Layout
await page.getByRole('button', { name: 'Save' }).click();
await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();

// Reload page and set Styles to 'None'
await page.reload({ waitUntil: 'domcontentloaded' });

// Edit Flexible Layout
await page.getByLabel('Edit').click();

// Select styles tab
await page.getByRole('tab', { name: 'Styles' }).click();

await setStyles(page, 'No Style', 'No Style', page.getByLabel('Stacked Plot Frame'));

// Check styles using checkStyles function
await checkStyles(
page,
hexToRGB(inheritedBackgroundColor),
hexToRGB(inheritedColor),
page.getByLabel('Stacked Plot Frame')
);
// Save Flexible Layout
await page.locator('button[title="Save"]').click();
await page.locator('text=Save and Finish Editing').click();

// Reload page and verify that styles persist
await page.reload({ waitUntil: 'domcontentloaded' });

// Check styles using checkStyles function
await checkStyles(
page,
hexToRGB(inheritedBackgroundColor),
hexToRGB(inheritedColor),
page.getByLabel('Stacked Plot Frame')
);
});
});

/**
* Converts a hex color value to its RGB equivalent.
*
* @param {string} hex - The hex color value.
* @returns {string} The RGB equivalent of the hex color.
*/
function hexToRGB(hex) {
const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex);
return result
? `rgb(${parseInt(result[1], 16)}, ${parseInt(result[2], 16)}, ${parseInt(result[3], 16)})`
: null;
}

/**
* Sets the background and text color of a given element.
*
* @param {object} page - The Playwright page object.
* @param {string} backgroundColorHex - The hex value of the background color to set.
* @param {string} textColorHex - The hex value of the text color to set.
* @param {object} locator - The Playwright locator for the element whose style is to be set.
*/
async function setStyles(page, backgroundColorHex, textColorHex, locator) {
await locator.click(); // Assuming the locator is clickable and opens the style setting UI
await page.getByLabel('Set background color').click();
await page.getByLabel(backgroundColorHex).click();
await page.getByLabel('Set text color').click();
await page.getByLabel(textColorHex).click();
}

/**
* Checks if the styles of an element match the expected values.
*
* @param {object} page - The Playwright page object.
* @param {string} expectedBackgroundColor - The expected background color in RGB format.
* @param {string} expectedTextColor - The expected text color in RGB format.
* @param {object} locator - The Playwright locator for the element whose style is to be checked.
*/
async function checkStyles(page, expectedBackgroundColor, expectedTextColor, locator) {
const layoutStyles = await locator.evaluate((el) => {
return {
background: window.getComputedStyle(el).getPropertyValue('background-color'),
fontColor: window.getComputedStyle(el).getPropertyValue('color')
};
});

expect(layoutStyles.background).toContain(expectedBackgroundColor);
expect(layoutStyles.fontColor).toContain(expectedTextColor);
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
"test:e2e:visual:ci": "percy exec --config ./e2e/.percy.ci.yml --partial -- npx playwright test --config=e2e/playwright-visual-a11y.config.js --project=chrome --grep-invert @unstable",
"test:e2e:visual:full": "percy exec --config ./e2e/.percy.nightly.yml -- npx playwright test --config=e2e/playwright-visual-a11y.config.js --grep-invert @unstable",
"test:e2e:full": "npx playwright test --config=e2e/playwright-ci.config.js --grep-invert @couchdb",
"test:e2e:watch": "npx playwright test --ui --config=e2e/playwright-ci.config.js",
"test:e2e:watch": "npx playwright test --ui --config=e2e/playwright-watch.config.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive-by: improve the ability to debug multiple tests at once

"test:perf:contract": "npx playwright test --config=e2e/playwright-performance-dev.config.js",
"test:perf:localhost": "npx playwright test --config=e2e/playwright-performance-prod.config.js --project=chrome",
"test:perf:memory": "npx playwright test --config=e2e/playwright-performance-prod.config.js --project=chrome-memory",
Expand Down
Loading