-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Flexible Layouts] Flexible Layout styling fixes #7319
Conversation
…elections on save, disable styling on flexible layout containers, and fix styling on plots on flexible layouts
Current Playwright Test Results Summary✅ 15 Passing Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 01/04/2024 01:05:09am UTC) Run DetailsRunning Workflow e2e-couchdb on Github Actions Commit: 8cf081d Started: 01/04/2024 01:02:35am UTC Current Playwright Test Results Summary✅ 172 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 01/04/2024 01:05:10am UTC)
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Telemetry Table unpauses and filters data when paused by button and user changes bounds
Retry 1 • Initial Attempt |
1.75% (1)1 / 57 runfailed over last 7 days |
28.07% (16)16 / 57 runsflaked over last 7 days |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7319 +/- ##
==========================================
- Coverage 55.70% 55.61% -0.09%
==========================================
Files 656 657 +1
Lines 26192 26239 +47
Branches 2546 2549 +3
==========================================
+ Hits 14589 14594 +5
- Misses 10898 10934 +36
- Partials 705 711 +6
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
/** | ||
* This test is dedicated to test styling of various plugins | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created this file as a general one that could be used for all "styling" related tests. Should I instead make this plugin specific (flexible layouts in this case)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no this is great
@@ -29,7 +29,7 @@ | |||
</div> | |||
|
|||
<div | |||
class="c-fl__container-holder" | |||
class="c-fl__container-holder u-style-receiver js-style-receiver" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to fix styles not being applied properly
|
||
if (layoutItem) { | ||
return true; | ||
} | ||
|
||
if (!domainObject) { | ||
if (!domainObject || isFlexibleLayoutContainer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to disable the styles tab on flexible layout containers
@@ -23,7 +23,7 @@ | |||
<template> | |||
<div | |||
v-if="loaded" | |||
class="c-plot c-plot--stacked holder holder-plot has-control-bar" | |||
class="c-plot c-plot--stacked holder holder-plot has-control-bar u-style-receiver js-style-receiver" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to fix the styles landing on the plot instead of the container. This created an issue where removing styles wasn't working properly since the add and remove targeted different elements.
# vue-eslint update 2019 | ||
14a0f84c1bcd56886d7c9e4e6afa8f7d292734e5 | ||
# eslint changes 2022 | ||
d80b6923541704ab925abf0047cbbc58735c27e2 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is so cool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! We should merge in the changes from master
and update our imports/exports to be ESM syntax. Other than that, LGTM!
/** | ||
* Sets the background and text color of a given element. | ||
* | ||
* @param {import('@playwright/test').Page} page - The Playwright page object. | ||
* @param {string} borderColorHex - The hex value of the border color to set, or 'No Style'. | ||
* @param {string} backgroundColorHex - The hex value of the background color to set, or 'No Style'. | ||
* @param {string} textColorHex - The hex value of the text color to set, or 'No Style'. | ||
* @param {import('@playwright/test').Locator} locator - The Playwright locator for the element whose style is to be set. | ||
*/ | ||
async function setStyles(page, borderColorHex, backgroundColorHex, textColorHex, locator) { | ||
await locator.click(); // Assuming the locator is clickable and opens the style setting UI | ||
await page.getByLabel('Set border color').click(); | ||
await page.getByLabel(borderColorHex).click(); | ||
await page.getByLabel('Set background color').click(); | ||
await page.getByLabel(backgroundColorHex).click(); | ||
await page.getByLabel('Set text color').click(); | ||
await page.getByLabel(textColorHex).click(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this rocks
Closes #6693
Describe your changes:
All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist