fix(#7338): Persisted styles correctly apply to `StackedPlotItem`s on mount #7341

ozyx · 2024-01-04T07:17:05Z

Styles weren't being applied to StackedPlotItems correctly because we would manually mount them (using Vue.createApp). This PR updates StackedPlotItem to use Plots directly in the template (since only Plots end up in StackedPlots anyway, we don't need to do anything dynamic).

Also fixes a small inconsistency where staleness styling wasn't being applied over custom border color styles for Stacked Plot Items.

Describe your changes:

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

Changes address original issue?
Tests included and/or updated with changes?
Has this been smoke tested?
Have you associated this PR with a type: label? Note: this is not necessarily the same as the original issue.
Have you associated a milestone with this PR? Note: leave blank if unsure.
Is this a breaking change to be called out in the release notes?
Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

Changes appear to address issue?
Reviewer has tested changes by following the provided instructions?
Changes appear not to be breaking changes?
Appropriate automated tests included?
Code style and in-line documentation are appropriate?

deploysentinel · 2024-01-04T07:21:42Z

Current Playwright Test Results Summary

✅ 15 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 01/08/2024 09:31:52am UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: b952990

Started: 01/08/2024 09:29:59am UTC

⚠️ Flakes

📄 functional/tree.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Main Tree Creating a child object on one tab and expanding its parent on the other shows the correct composition @couchdb @2p Retry 1 • Initial Attempt	0% (0) 0 / 23 runs failed over last 7 days	4.35% (1) 1 / 23 run flaked over last 7 days

View Detailed Build Results

Current Playwright Test Results Summary

✅ 173 Passing - ⚠️ 4 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 01/08/2024 09:31:52am UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: b952990

Started: 01/04/2024 09:25:22am UTC

⚠️ Flakes

📄 functional/plugins/styling/stackedPlotStyling.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Stacked Plot styling styling the overlay plot properly applies the styles to all containers Retry 1 • Initial Attempt	31.58% (12) 12 / 38 runs failed over last 7 days	2.63% (1) 1 / 38 run flaked over last 7 days

📄 functional/plugins/plot/logPlot.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Log plot tests Log Plot ticks are functionally correct in regular and log mode and after refresh Retry 2 • Retry 1 • Initial Attempt	1.82% (1) 1 / 55 run failed over last 7 days	20% (11) 11 / 55 runs flaked over last 7 days

📄 functional/planning/timelist.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Time List Create a Time List, add a single Plan to it and verify all the activities are displayed with no milliseconds Retry 1 • Initial Attempt	2.08% (1) 1 / 48 run failed over last 7 days	60.42% (29) 29 / 48 runs flaked over last 7 days

📄 functional/plugins/notebook/restrictedNotebook.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Restricted Notebook with a page locked and with an embed @addinit Disallows embeds to be deleted if page locked @addinit Retry 1 • Initial Attempt	4.08% (2) 2 / 49 runs failed over last 7 days	53.06% (26) 26 / 49 runs flaked over last 7 days

View Detailed Build Results

e2e/tests/functional/plugins/styling/stackedPlotStyling.e2e.spec.js

unlikelyzero · 2024-01-04T07:28:29Z

e2e/tests/functional/plugins/styling/stackedPlotStyling.e2e.spec.js

+      hexToRGB(setBorderColor),
+      hexToRGB(setBackgroundColor),
+      hexToRGB(setTextColor),
+      page.getByLabel('Plot Container Style Target').first()


codecov · 2024-01-04T07:31:53Z

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (70de736) 55.78% compared to head (b952990) 55.46%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7341      +/-   ##
==========================================
- Coverage   55.78%   55.46%   -0.32%     
==========================================
  Files         657      657              
  Lines       26203    26173      -30     
  Branches     2549     2546       -3     
==========================================
- Hits        14618    14518     -100     
- Misses      10881    10954      +73     
+ Partials      704      701       -3

Flag	Coverage Δ		*Carryforward flag
e2e-full	`41.04% <ø> (-0.02%)`	⬇️	Carriedforward from 930f95f
e2e-stable	`58.94% <ø> (+0.16%)`	⬆️
unit	`48.30% <0.00%> (-0.53%)`	⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files	Coverage Δ
src/plugins/plot/PlotView.vue	`69.81% <ø> (+1.29%)`	⬆️
src/plugins/plot/stackedPlot/StackedPlotItem.vue	`22.38% <0.00%> (-15.12%)`	⬇️

... and 22 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70de736...b952990. Read the comment docs.

ozyx · 2024-01-04T09:17:22Z

src/plugins/plot/PlotView.vue

@@ -78,7 +85,7 @@ export default {
    PlotLegend
  },
  mixins: [stalenessMixin],
-  inject: ['openmct', 'domainObject', 'path'],
+  inject: ['openmct', 'domainObject'],


this isn't used so i removed it

ozyx · 2024-01-04T09:19:32Z

src/plugins/plot/stackedPlot/StackedPlotItem.vue

    };
  },
-  watch: {


no more need for this since we pass them all as props directly

ozyx · 2024-01-04T09:21:59Z

src/plugins/plot/stackedPlot/StackedPlotItem.vue

-        this.subscribeToStaleness(object, (stalenessResponse) => {
-          this.updateComponentProp('isStale', stalenessResponse.isStale);
-        });
+        this.subscribeToStaleness(object);


we don't need to set with the callback since the mixin already provides and updates isStale

unlikelyzero · 2024-01-07T18:53:13Z

src/plugins/plot/stackedPlot/StackedPlotItem.vue

+      openmct: this.openmct,
+      domainObject: this.childObject
+    };
+  },


why isn't this line covered

coverage metrics are out of whack

scottbell

Love all the deleted code @ozyx - good work!

Before:

Screen.Recording.2024-01-08.at.10.43.06.AM.mov

After:

Screen.Recording.2024-01-08.at.10.41.42.AM.mov

ozyx added 5 commits January 3, 2024 22:32

refactor: use Plot component directly in StackedPlotItem template

60829f2

test(e2e): fix style test for stackedplot

b3c0274

test(e2e): add annotation back in

3c1c73e

test: fix unit tests

2fca66d

refactor: tidy up

930f95f

unlikelyzero reviewed Jan 4, 2024

View reviewed changes

e2e/tests/functional/plugins/styling/stackedPlotStyling.e2e.spec.js Outdated Show resolved Hide resolved

unlikelyzero reviewed Jan 4, 2024

View reviewed changes

e2e/tests/functional/plugins/styling/stackedPlotStyling.e2e.spec.js Outdated Show resolved Hide resolved

unlikelyzero reviewed Jan 4, 2024

View reviewed changes

ozyx added 2 commits January 4, 2024 01:04

fix: move const, remove eslint ignore

64857a2

fix: apply staleness styling properly

fafc6b4

ozyx added the type:bug label Jan 4, 2024

ozyx added this to the Target:3.3.0 milestone Jan 4, 2024

ozyx commented Jan 4, 2024

View reviewed changes

refactor: remove unused data()

8bae1f9

ozyx commented Jan 4, 2024

View reviewed changes

refactor: remove unused isMissing

9d5aeab

ozyx commented Jan 4, 2024

View reviewed changes

refactor: remove debug statements

b952990

ozyx marked this pull request as ready for review January 4, 2024 09:31

ozyx requested review from khalidadil, charlesh88 and scottbell January 4, 2024 15:52

unlikelyzero self-requested a review January 7, 2024 18:51

unlikelyzero approved these changes Jan 7, 2024

View reviewed changes

unlikelyzero reviewed Jan 7, 2024

View reviewed changes

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Jan 8, 2024

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Jan 8, 2024

scottbell approved these changes Jan 8, 2024

View reviewed changes

ozyx merged commit dfba4e2 into master Jan 8, 2024
29 of 30 checks passed

ozyx deleted the mct7338 branch January 8, 2024 18:19

unlikelyzero modified the milestones: Target:3.3.0, Target:4.0.0 Jan 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(#7338): Persisted styles correctly apply to `StackedPlotItem`s on mount #7341

fix(#7338): Persisted styles correctly apply to `StackedPlotItem`s on mount #7341

ozyx commented Jan 4, 2024 •

edited

Loading

deploysentinel bot commented Jan 4, 2024 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

Test Case Results

Test Case Results

unlikelyzero Jan 4, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading

ozyx Jan 4, 2024

ozyx Jan 4, 2024

ozyx Jan 4, 2024

unlikelyzero Jan 7, 2024

ozyx Jan 7, 2024

scottbell left a comment

fix(#7338): Persisted styles correctly apply to StackedPlotItems on mount #7341

fix(#7338): Persisted styles correctly apply to StackedPlotItems on mount #7341

Conversation

ozyx commented Jan 4, 2024 • edited Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

deploysentinel bot commented Jan 4, 2024 • edited Loading

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Test Case Results

Test Case Results

unlikelyzero Jan 4, 2024

Choose a reason for hiding this comment

codecov bot commented Jan 4, 2024 • edited Loading

Codecov Report

ozyx Jan 4, 2024

Choose a reason for hiding this comment

ozyx Jan 4, 2024

Choose a reason for hiding this comment

ozyx Jan 4, 2024

Choose a reason for hiding this comment

unlikelyzero Jan 7, 2024

Choose a reason for hiding this comment

ozyx Jan 7, 2024

Choose a reason for hiding this comment

scottbell left a comment

Choose a reason for hiding this comment

fix(#7338): Persisted styles correctly apply to `StackedPlotItem`s on mount #7341

fix(#7338): Persisted styles correctly apply to `StackedPlotItem`s on mount #7341

ozyx commented Jan 4, 2024 •

edited

Loading

deploysentinel bot commented Jan 4, 2024 •

edited

Loading

codecov bot commented Jan 4, 2024 •

edited

Loading