UI updates to Y axis #6102

shefalijoshi · 2023-01-06T20:39:23Z

Describe your changes:

Known Issues:
Adding/Removing series to different Y axis buckets requires navigating away and back to the overlay plot.

Changes:

Move the Y axis name to the bottom of the y-axis area
Move the gear icon below the name of the y-axis
Add a visibility toggle to the y-axis
Add color swatches to indicate which series are part of the y axis (no hover interaction is done for MVP)

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?
Command line build passes?
Has this been smoke tested?
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?
Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

* Set yKey on each model * Add a test

* Update version for sprint 2.1.1 * Basic changes (very draft) for multiple y axis for plots * Generalize additional axes model handling * [Plot] Set yKey on each plot series (#5790) * Set yKey on each model * Add a test * Update yaxis to be independent of mctplot * Fix yAxis ticks * Fix yAxisModel label * Add multiple y axes option for overlay plot. UI changes to display 2 y-axes * Fix Chart bug * Bump version to `2.1.3` (#5973) * Preserve Gauge configuration changes on create/edit (#5986) * fix(#5985): deep merge on create/edit properties - Perform a deep merge of old and new properties on Create/Edit properties actions * refactor(e2e): improve selector in appActions * test(e2e): add tests for gauges - test creating a non-default gauge (checks only for console errors) - test updating a gauge (checks only for console errors) * fix(e2e): use pluginFixtures for gauge tests * fix(e2e): prevent fail if testNotes is undefined * Make the tree key unique (#5989) * Refactor iterating over all yAxis while drawing to the viewport * Refactor code to remove hardcoding of yAxisIds * style: auto-fix lint errors * Remove configurability of number of y axes. Use yAxisIds from Series to show/hide axes. Adds test for multiple y axis display * Add more tests for when only 1 axis needs to be displayed * Address code review comments: Move styles to computed properties Refactor yAxes rendering in MctPlot to also include main y axis. * Address review comments. Refactor plot configuration code for additiona y axes and reset the chart only for series that belong to the yaxis that has changed. * Fix bug where only the first series for a given yAxis was rendering * Refactor code to reuse method logic, move const and use cap case * Only draw highlights for the given yAxis * Only draw rectangles for the yAxis the series belongs to Co-authored-by: Khalid Adil <[email protected]> Co-authored-by: Jesse Mazzella <[email protected]> Co-authored-by: Jesse Mazzella <[email protected]>

…sa/openmct into feature/plots-multiple-y-axes

Fix dropdown being obscured for the right y-axis

codecov · 2023-01-06T20:44:29Z

Codecov Report

❗ No coverage uploaded for pull request base (feature/plots-multiple-y-axes@97ae390). Click here to learn what that means.
The diff coverage is n/a.

@@                       Coverage Diff                        @@
##             feature/plots-multiple-y-axes    #6102   +/-   ##
================================================================
  Coverage                                 ?   55.04%           
================================================================
  Files                                    ?      602           
  Lines                                    ?    24223           
  Branches                                 ?     2187           
================================================================
  Hits                                     ?    13333           
  Misses                                   ?    10271           
  Partials                                 ?      619

Flag	Coverage Δ	*Carryforward flag
e2e-ci	`39.49% <0.00%> (?)`	Carriedforward from e24aaff
e2e-full	`51.23% <0.00%> (?)`	Carriedforward from e24aaff
e2e-stable	`50.25% <0.00%> (?)`
unit	`50.92% <0.00%> (?)`

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

Continue to review full report at Codecov.

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

shefalijoshi · 2023-01-06T20:49:41Z

Testing Notes:
Create an overlay plot
Add multiple telemetry endpoints to the overlay plot
In edit mode, scroll to the Elements Pool and drag/drop series to different Y axis Note: you may need to navigate away from the plot to see the changes
Play with it - have fun!

rukmini-bose

Review Comments:
This looks great! Super duper impressive. Tested on regular overlay plots, as well as checked how it looks in a stacked plot.

Couple of things:

1. The visibility toggle button seems to be faulty. Check out the video– SWG C (purple plot) seems to be re-rendering even after we hide its respective Y Axis.

Multiple.Y.Axes.mov

2. Noticing that the Y Axis 2 (furthest from the left) is always cut off in the canvas. Can we fix this?

3. Something odd is happening over here with Y Axis 1 and 2. I think that the two axes are being overlapped over each other.

4. For Y-Axis 3, it would be nice if we could have the ticks closer to the canvas rather than the label. That way, there is less of a space between the plot and ticks, so it is easier for users to visualize the hash lines to the ticks. Plus, it'll be more consistent with the other axes, who have the ticks next to plot.

5. Dragging and dropping elements between axes in the elements pool is difficult and unintuitive at times– specifically when you are trying to drop a new element into an axis that is already populated by another axis. We need to work on making the highlights that appear on drag more clear. UPDATE: After talking with @ozyx , we have decided to create a separate and new element for objects in the elements pool for multiple y-axes. This change will remove the grippy element, which is causing the issues I encountered when dragging elements between axes.
6. Add instructions into the elements pool to make it clearer to users how to interact with it.

…not allowing for proper drag and drop

…t y axis

…ment

rukmini-bose

This all looks super amazing. Everything looks great one the regular overlay plot object.

My only observation now is that the axes do not line up properly when there are multiple y axes plots being used in a stacked plot.
We would need to calculate which plot within the stacked plot has the maximum width, and make sure that the rest of the plots within the stacked plot take on that width as well. That'll ensure that the plots (and their x-axis) line up properly, which is important in a stacked plot.

rukmini-bose

Looks good! There's a bug with how multiple y axes looks in stacked plots, but I'll file a bug for it. Approving now. 🥳

* Add color swatches for Y Axis * Allow showing and hiding plots for a given yaxis. Fix dropdown being obscured for the right y-axis * Show visibility icon only for non simple plots * Add instruction message to Elements Pool * Add grippy class to object label. Fixes issue where using grippy was not allowing for proper drag and drop * Fix clipping of text on left y axis. Fix positioning of ticks on right y axis * Fix arguments to filter that was causing hidden series to be drawn * test(e2e): fix test, better expect conditions Co-authored-by: Khalid Adil <[email protected]> Co-authored-by: Jesse Mazzella <[email protected]> Co-authored-by: Jesse Mazzella <[email protected]> Co-authored-by: Rukmini Bose <[email protected]>

khalidadil and others added 6 commits December 1, 2022 15:59

[Plot] Set yKey on each plot series (#5790)

1c8d8a7

* Set yKey on each model * Add a test

Merge branch 'feature/plots-multiple-y-axes' of https://github.com/na…

9f220ca

…sa/openmct into feature/plots-multiple-y-axes

Merge branch 'feature/plots-multiple-y-axes' of https://github.com/na…

c1e94e6

…sa/openmct into feature/plots-multiple-y-axes

Add color swatches for Y Axis

8e6a844

Allow showing and hiding plots for a given yaxis.

7ce868c

Fix dropdown being obscured for the right y-axis

shefalijoshi requested review from rukmini-bose and charlesh88 January 6, 2023 20:39

Show visibility icon only for non simple plots

f028bbc

ozyx added this to the Feature/MultiYAxis milestone Jan 17, 2023

rukmini-bose requested changes Jan 17, 2023

View reviewed changes

Add instruction message to Elements Pool

ec5524f

rukmini-bose self-requested a review January 18, 2023 01:22

ozyx mentioned this pull request Jan 19, 2023

[Multiple Y-Axes] feat(ElementItem): optionally hide grippy #6134

Closed

15 tasks

rukmini-bose and others added 4 commits January 18, 2023 17:10

Add grippy class to object label. Fixes issue where using grippy was …

838259f

…not allowing for proper drag and drop

Fix clipping of text on left y axis. Fix positioning of ticks on righ…

a8f7068

…t y axis

Fix arguments to filter that was causing hidden series to be drawn

4b1878b

Merge branch 'feature/plots-multiple-y-axes' into y-axis-visual-treat…

e24aaff

…ment

rukmini-bose requested changes Jan 20, 2023

View reviewed changes

shefalijoshi requested a review from rukmini-bose January 20, 2023 18:01

rukmini-bose approved these changes Jan 20, 2023

View reviewed changes

rukmini-bose mentioned this pull request Jan 20, 2023

[Stacked Plots] Plots are not lining up properly with Multiple Y Axes in Stacked Plots #6147

Closed

7 tasks

test(e2e): fix test, better expect conditions

2d9f1b8

ozyx merged commit f0f0e1d into feature/plots-multiple-y-axes Jan 20, 2023

ozyx deleted the y-axis-visual-treatment branch January 20, 2023 19:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI updates to Y axis #6102

UI updates to Y axis #6102

shefalijoshi commented Jan 6, 2023 •

edited

Loading

codecov bot commented Jan 6, 2023 •

edited

Loading

shefalijoshi commented Jan 6, 2023

rukmini-bose left a comment •

edited by shefalijoshi

Loading

rukmini-bose left a comment

rukmini-bose left a comment

UI updates to Y axis #6102

UI updates to Y axis #6102

Conversation

shefalijoshi commented Jan 6, 2023 • edited Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

codecov bot commented Jan 6, 2023 • edited Loading

Codecov Report

shefalijoshi commented Jan 6, 2023

rukmini-bose left a comment • edited by shefalijoshi Loading

Choose a reason for hiding this comment

rukmini-bose left a comment

Choose a reason for hiding this comment

rukmini-bose left a comment

Choose a reason for hiding this comment

shefalijoshi commented Jan 6, 2023 •

edited

Loading

codecov bot commented Jan 6, 2023 •

edited

Loading

rukmini-bose left a comment •

edited by shefalijoshi

Loading