fix(#6503): Recently Viewed Items - Disable button if no items #6533

vhenckel · 2023-03-30T22:05:55Z

Closes #6503

Describe your changes:

Disabling button when recent view list is empty.

Screen.Recording.2023-03-30.at.19.03.57.mov

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)

codecov · 2023-03-30T22:15:46Z

Codecov Report

Merging #6533 (4d66b40) into master (344bf8e) will decrease coverage by 0.08%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #6533      +/-   ##
==========================================
- Coverage   54.77%   54.69%   -0.08%     
==========================================
  Files         628      628              
  Lines       26798    26802       +4     
  Branches     2420     2421       +1     
==========================================
- Hits        14678    14659      -19     
- Misses      11453    11478      +25     
+ Partials      667      665       -2

Flag	Coverage Δ		*Carryforward flag
e2e-ci	`39.34% <ø> (ø)`		Carriedforward from 15bee18
e2e-full	`51.06% <ø> (ø)`		Carriedforward from 15bee18
e2e-stable	`55.46% <ø> (-0.02%)`	⬇️
unit	`48.91% <75.00%> (-0.08%)`	⬇️	Carriedforward from 15bee18

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

Impacted Files	Coverage Δ
src/ui/layout/RecentObjectsList.vue	`66.12% <66.66%> (+0.02%)`	⬆️
src/ui/layout/Layout.vue	`20.40% <100.00%> (+1.65%)`	⬆️

... and 8 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 344bf8e...4d66b40. Read the comment docs.

unlikelyzero · 2023-03-31T03:45:31Z

@vhenckel would you be able to add a quick e2e test for this?

/openmct/e2e/tests/functional/recentObjects.e2e.spec.js

vhenckel · 2023-03-31T15:26:01Z

@vhenckel would you be able to add a quick e2e test for this?

/openmct/e2e/tests/functional/recentObjects.e2e.spec.js

@unlikelyzero add e2e test.

ozyx

Great work! I've left just a few suggestions.

e2e/tests/functional/recentObjects.e2e.spec.js

src/ui/layout/Layout.vue

… the button becomes active again

ozyx

Thanks for the quick update! We're getting close. I've got just a few more change requests:

e2e/tests/functional/recentObjects.e2e.spec.js

src/ui/layout/RecentObjectsList.vue

src/ui/layout/Layout.vue

ozyx · 2023-03-31T20:26:07Z

e2e/tests/functional/recentObjects.e2e.spec.js

+        expect(await recentObjectsList.locator('.c-recentobjects-listitem').count()).toBe(3);
+
+        // Assert that the button is enabled
+        expect(


ozyx

Found one issue-- we should disable the clear button only after the user has confirmed the clear action.

src/ui/layout/Layout.vue

…t of recents

ozyx

LGTM. Smoke-tested this and it works well.
Excellent work, thank you!

Quick sidenote:
I went ahead and fixed the failing e2e test (it was failing because we were already focused on the clock object when navigating to it, so Recent Objects did not populate). Also, our initial state assertions were just checking for toBeTruthy(), which evaluates to true whether or not the locator exists-- I switched those to toBeVisible().

vhenckel added 2 commits March 30, 2023 19:02

fix(nasa#6503): Recently Viewed Items - Disable button if no items

57e459c

Merge branch 'master' into mct6503

0bda7b5

ozyx added the source:community Community contribution or request label Mar 30, 2023

ozyx self-requested a review March 30, 2023 22:37

ozyx added this to the Target: 2.2.2 milestone Mar 30, 2023

unlikelyzero self-requested a review March 31, 2023 03:46

vhenckel added 2 commits March 31, 2023 12:24

Merge branch 'master' into mct6503

4896a47

add e2e test

b21abcf

ozyx suggested changes Mar 31, 2023

View reviewed changes

vhenckel added 6 commits March 31, 2023 14:42

fix: remove slow function

a878195

test: After deactivating the button, new objects must be inserted and…

a1ace6e

… the button becomes active again

test: After deactivating the button, new objects must be inserted and…

bb4feda

… the button becomes active again

test: ensure clear recent objects button is active or inactive

b7a3956

add an event to notify when an object is inserted in the recents list

ed14b07

add an event to notify when an object is inserted in the recents list

7cb2c76

ozyx suggested changes Mar 31, 2023

View reviewed changes

vhenckel added 4 commits March 31, 2023 17:45

fix: adjusting function name and add validation for triggering the event

f69d834

fix: adjusting function name and add validation for triggering the event

7b5803d

fix: adjusting function name and add validation for triggering the event

f112679

Merge branch 'master' into mct6503

6b7641a

ozyx suggested changes Mar 31, 2023

View reviewed changes

src/ui/layout/Layout.vue Outdated Show resolved Hide resolved

src/ui/layout/Layout.vue Outdated Show resolved Hide resolved

vhenckel added 2 commits April 1, 2023 13:59

Merge branch 'master' into mct6503

bc3248f

fix: add event to disable button only when user click ok to clear lis…

8a682a4

…t of recents

vhenckel requested a review from ozyx April 3, 2023 14:14

test: fix failing e2e + better assertions

38ad8c2

ozyx approved these changes Apr 3, 2023

View reviewed changes

ozyx enabled auto-merge (squash) April 3, 2023 17:13

ozyx added 2 commits April 3, 2023 10:13

Merge branch 'master' into mct6503

15bee18

Merge branch 'master' into mct6503

4d66b40

ozyx merged commit bc3a540 into nasa:master Apr 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(#6503): Recently Viewed Items - Disable button if no items #6533

fix(#6503): Recently Viewed Items - Disable button if no items #6533

vhenckel commented Mar 30, 2023 •

edited

Loading

codecov bot commented Mar 30, 2023 •

edited

Loading

unlikelyzero commented Mar 31, 2023

vhenckel commented Mar 31, 2023

ozyx left a comment

ozyx left a comment

ozyx Mar 31, 2023

ozyx left a comment

ozyx left a comment

fix(#6503): Recently Viewed Items - Disable button if no items #6533

fix(#6503): Recently Viewed Items - Disable button if no items #6533

Conversation

vhenckel commented Mar 30, 2023 • edited Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

codecov bot commented Mar 30, 2023 • edited Loading

Codecov Report

unlikelyzero commented Mar 31, 2023

vhenckel commented Mar 31, 2023

ozyx left a comment

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

ozyx Mar 31, 2023

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

vhenckel commented Mar 30, 2023 •

edited

Loading

codecov bot commented Mar 30, 2023 •

edited

Loading