Add collapse all and expand all buttons to Tree component #574

kinow · 2021-01-19T02:37:08Z

These changes close #246

Requirements check-list

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Appropriate tests are included (unit and/or functional).
Appropriate change log entry included.
No documentation update required.

kinow · 2021-01-19T02:39:32Z

Today I just did the template after reading the design issue, tomorrow will do more testing with a real workflow.

@oliver-sanders, @hjoliver I couldn't find where to put the collapse/expand all buttons in the design issue. Any suggestions?

Thanks!

codecov-io · 2021-01-19T02:41:37Z

Codecov Report

Merging #574 (063acc8) into master (6f1dfcb) will decrease coverage by 0.69%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #574      +/-   ##
==========================================
- Coverage   81.66%   80.97%   -0.70%     
==========================================
  Files          66       45      -21     
  Lines        1320     1072     -248     
  Branches       81       71      -10     
==========================================
- Hits         1078      868     -210     
+ Misses        223      185      -38     
  Partials       19       19

Flag	Coverage Δ
e2e	`?`
unittests	`80.97% <ø> (+1.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/components/cylc/gscan/GScan.vue	`96.66% <ø> (-3.34%)`	⬇️
src/components/cylc/tree/Tree.vue	`72.72% <ø> (+10.60%)`	⬆️
src/components/cylc/workflow/Lumino.vue	`38.70% <ø> (-54.84%)`	⬇️
src/components/cylc/workflow/lumino-widget.js	`0.00% <0.00%> (-84.22%)`	⬇️
src/App.vue	`54.54% <0.00%> (-36.37%)`	⬇️
src/components/cylc/tree/TreeItem.vue	`73.07% <0.00%> (-23.08%)`	⬇️
.../components/graphqlFormGenerator/FormGenerator.vue	`69.56% <0.00%> (-13.05%)`	⬇️
src/services/gquery.js	`61.29% <0.00%> (-11.30%)`	⬇️
... and 24 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 6f1dfcb...7b73df5. Read the comment docs.

hjoliver · 2021-01-19T21:04:45Z

@kinow I think we'd want expand-all to stop at the task level. i.e. expand all task families, but don't expand the jobs (the icons show how many jobs there are anyway) and definitely don't expand the job details leaves.

kinow · 2021-01-20T00:24:43Z

@kinow I think we'd want expand-all to stop at the task level. i.e. expand all task families, but don't expand the jobs (the icons show how many jobs there are anyway) and definitely don't expand the job details leaves.

Done! The function already supported a filter function :-) so just passed a filter that looks for the types we don't want to expand (job and job-details). Didn't change the collapse all as I think it should be OK to collapse everything, including jobs & details.

hjoliver · 2021-01-20T05:12:01Z

The button positions seem good to me.

Noticed a couple of issues though, from a quick play:

expand-all is still expanding the task nodes so that each job is listed on a new line. IMO the expansion should probably stop at tasks. That's enough to see every task, and the job icons, without using up a lot more screen lines than necessary (thinking of large flows).
~~when a new cycle point is added it appears in the expanded, even if collapse-all was pressed~~ (scratch that, I probably collapsed cycle points manually before the new one was added)
shrinking the window width, the minus drops under the plus, then they both disappear off the end

kinow · 2021-01-20T05:25:56Z

expand-all is still expanding the task nodes so that each job is listed on a new line. IMO the expansion should probably stop at tasks. That's enough to see every task, and the job icons, without using up a lot more screen lines than necessary (thinking of large flows).

Ah, sorry, I misunderstood it. That'll be easy to fix.

shrinking the window width, the minus drops under the plus, then they both disappear off the end

👍 will make it a flex element, and adjust how it behaves (wrap, justify, etc). I'm more concerned about the location, whether the icon is OK (I used the first that came to my mind).

And the feature could be a bit different. We could make it stateful. So if you click expand all, then every subsequent delta will be expanded too. Meaning that the UI keeps the "expand all" state, until you click the expand all icon again. That can be tricky to implement/test, but new deltas would be expanded too.

Thanks @hjoliver !

oliver-sanders · 2021-01-20T17:14:51Z

@oliver-sanders, @hjoliver I couldn't find where to put the collapse/expand all buttons in the design issue. Any suggestions?

There is an issue for this, although the answer is pretty much "where you've put them". #471

kinow · 2021-01-20T21:20:07Z

@oliver-sanders, @hjoliver I couldn't find where to put the collapse/expand all buttons in the design issue. Any suggestions?

There is an issue for this, although the answer is pretty much "where you've put them". #471

Thanks @oliver-sanders !

kinow · 2021-01-20T23:50:53Z

tests/unit/components/cylc/tree/tree.vue.spec.js

+    }
+  ]
+}
+localVue.use(CylcObjectPlugin)


The code above removes several warnings from the console about missing the directive for cylc-object.

kinow · 2021-01-20T23:51:29Z

tests/unit/components/cylc/tree/tree.vue.spec.js

@@ -39,7 +58,7 @@ describe('Tree component', () => {
  // mount function from Vuetify docs https://vuetifyjs.com/ru/getting-started/unit-testing/
  /**
   * @param options
-   * @returns {Tree}
+   * @returns {Wrapper<Tree>}


The wrapper has different attributes and functions, so the proper annotation gives the IDE better auto-complete (e.g. findAllComponents and displays its signature)

kinow · 2021-01-20T23:52:57Z

tests/unit/components/cylc/tree/tree.vue.spec.js

        // task proxy is displayed
-        expect(taskProxy.$children[0].filtered).to.equal(true)
+        expect(taskProxy.vm.filtered).to.equal(true)


These tests were broken!!!

I thought children would return the children components of Tree. And it does, but I thought the children were TreeItem. Turns out it brings several components, like the TreeItems, but also all the Vuetify components (like VTextField and whatnot).

So fixed it by using the Wrapper and WrapperArray objects functions (and with some debugging 😬 ).

They got broken in this PR as I added new children components for the plus and minus icons.

kinow · 2021-01-21T00:57:51Z

Still pending

prevent the plus and minus icons from wrapping
move the expand/collapse out of the filters area (i.e. out of element with `v-if='filterable')

kinow · 2021-01-24T22:13:44Z

expand-all is still expanding the task nodes so that each job is listed on a new line. IMO the expansion should probably stop at tasks. That's enough to see every task, and the job icons, without using up a lot more screen lines than necessary (thinking of large flows).

Last commit expands workflows, cycle points, and family proxies. Tasks, jobs, and job details won't be expanded.

kinow · 2021-01-25T03:36:32Z

@hjoliver I've fixed the plus and minus icons wrapping. However, I noticed that the filter elements (task search name, task statuses) are responsive. That means that when you are using a small viewport, they adjust to be displayed in a different way.

But when you have a small widget area, they are not responsive. Here's what I mean:

The filter elements are reducing the width, but I think they shouldn't. And there is no way to tell the filter or expand/collapse toggle that they need to be displayed in a different way. In this case, I think making a fixed width would fix it.

But, in the end, I believe this will be fixed when we adjust the filter to match the design toolbar @oliver-sanders mentioned earlier #471

Once we adopt that design, this issue will be gone. Note too, that this happens before this PR, that's why I stopped messing with layout/responsive/flexbox/etc. Otherwise this change would grow more, making it harder to review/revert/conflicts/etc.

WDYT?

hjoliver · 2021-01-26T01:05:34Z

WDYT?

Seems fine to me, as we have a plan that will fix it.

kinow · 2021-01-26T02:42:41Z

Started writing the tests for expand/collapse methods. Even though that's not part of this PR, as it had been implemented since the initial version of the tree component, only now we are using that code. Should be done by tomorrow, then this one will be ready for review.

kinow · 2021-01-27T00:32:40Z

Added unit test and changelog. Had some issues writing the tests because I used an array of test parameters (wish there was something like parametrized tests in mocha+chai), and was calling .expandAll and collapseAll, and getting confusing results.

Turns out these functions mutate the objects I was passing to the tests, so the first test mutated the data, the second mutated again, etc. Until one failed. 😥 Fixed with Lodash's mutate (because Object.assign is a shallow-copy).

kinow · 2021-02-08T20:51:07Z

Rebased, making sure I didn't accidentally re-introduce the enum.toLowerCase() after @hjoliver fixed that! 🥇 ready for review again.

oliver-sanders

Looks good, tested as working.

One quirk, the tooltip seems to appear in-line with the Lumino/Workflow component rather than the expand/collapse button.

kinow · 2021-02-14T20:16:05Z

Looks good, tested as working.

Thanks!

One quirk, the tooltip seems to appear in-line with the Lumino/Workflow component rather than the expand/collapse button.

Ah! Tested locally, and you are correct. The tooltip appears over the Add View link, giving the wrong impression that they are related somehow. I've added the bottom directive to the tooltip, and now it should always be under the "Collapse All", "Expand All" links (which should work well for responsive and desktop viewports)

…ents

kinow · 2021-02-14T20:17:53Z

Also rebased 👍

kinow · 2021-02-14T20:27:10Z

The CI failure is related to #590 not related to this PR

hjoliver

Works for me, nice 👍

kinow self-assigned this Jan 19, 2021

kinow added this to the 0.3 milestone Jan 19, 2021

kinow force-pushed the collapse-expand-all branch from 33be555 to ea389c8 Compare January 20, 2021 21:18

kinow force-pushed the collapse-expand-all branch from ea389c8 to 9ccf7ee Compare January 20, 2021 23:49

kinow commented Jan 20, 2021

View reviewed changes

kinow force-pushed the collapse-expand-all branch from 154633c to b6a2e5d Compare January 27, 2021 00:27

kinow marked this pull request as ready for review January 27, 2021 00:29

kinow force-pushed the collapse-expand-all branch from b6a2e5d to 68575bb Compare February 8, 2021 20:35

oliver-sanders approved these changes Feb 11, 2021

View reviewed changes

kinow and others added 5 commits February 15, 2021 09:16

Add collapse all and expand all buttons to Tree component

544c4c3

Do not expand jobs and jobs-details

b950f2e

Expand workflow, cycle point, and family proxy only

26bd721

Separate filters and expand-collapse toggle, prevent wrapping of elem…

56bee4e

…ents

Add padding to both sides of Lumino workflow (only left was showing)

6660789

kinow added 5 commits February 15, 2021 09:16

Hide expand-collapse toggle in GScan tree

3c30261

Reduce padding from 4 to 2

4eae69b

Add unit test for collapseAll and for expandAll

7c9410a

Changelog

effb700

Show tooltip below links

7b73df5

kinow force-pushed the collapse-expand-all branch from 063acc8 to 7b73df5 Compare February 14, 2021 20:16

hjoliver approved these changes Feb 14, 2021

View reviewed changes

hjoliver merged commit f9a4ac0 into cylc:master Feb 14, 2021

kinow deleted the collapse-expand-all branch February 14, 2021 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add collapse all and expand all buttons to Tree component #574

Add collapse all and expand all buttons to Tree component #574

kinow commented Jan 19, 2021 •

edited

Loading

kinow commented Jan 19, 2021 •

edited

Loading

codecov-io commented Jan 19, 2021 •

edited

Loading

hjoliver commented Jan 19, 2021

kinow commented Jan 20, 2021 •

edited

Loading

hjoliver commented Jan 20, 2021 •

edited

Loading

kinow commented Jan 20, 2021 •

edited

Loading

oliver-sanders commented Jan 20, 2021

kinow commented Jan 20, 2021

kinow Jan 20, 2021

kinow Jan 20, 2021

kinow Jan 20, 2021

kinow commented Jan 21, 2021

kinow commented Jan 24, 2021

kinow commented Jan 25, 2021

hjoliver commented Jan 26, 2021

kinow commented Jan 26, 2021

kinow commented Jan 27, 2021

kinow commented Feb 8, 2021

oliver-sanders left a comment

kinow commented Feb 14, 2021

kinow commented Feb 14, 2021

kinow commented Feb 14, 2021

hjoliver left a comment

Add collapse all and expand all buttons to Tree component #574

Add collapse all and expand all buttons to Tree component #574

Conversation

kinow commented Jan 19, 2021 • edited Loading

kinow commented Jan 19, 2021 • edited Loading

codecov-io commented Jan 19, 2021 • edited Loading

Codecov Report

hjoliver commented Jan 19, 2021

kinow commented Jan 20, 2021 • edited Loading

hjoliver commented Jan 20, 2021 • edited Loading

kinow commented Jan 20, 2021 • edited Loading

oliver-sanders commented Jan 20, 2021

kinow commented Jan 20, 2021

kinow Jan 20, 2021

Choose a reason for hiding this comment

kinow Jan 20, 2021

Choose a reason for hiding this comment

kinow Jan 20, 2021

Choose a reason for hiding this comment

kinow commented Jan 21, 2021

kinow commented Jan 24, 2021

kinow commented Jan 25, 2021

hjoliver commented Jan 26, 2021

kinow commented Jan 26, 2021

kinow commented Jan 27, 2021

kinow commented Feb 8, 2021

oliver-sanders left a comment

Choose a reason for hiding this comment

kinow commented Feb 14, 2021

kinow commented Feb 14, 2021

kinow commented Feb 14, 2021

hjoliver left a comment

Choose a reason for hiding this comment

kinow commented Jan 19, 2021 •

edited

Loading

kinow commented Jan 19, 2021 •

edited

Loading

codecov-io commented Jan 19, 2021 •

edited

Loading

kinow commented Jan 20, 2021 •

edited

Loading

hjoliver commented Jan 20, 2021 •

edited

Loading

kinow commented Jan 20, 2021 •

edited

Loading