Auto-flatten single-child directories in GScan sidebar #1416

MetRonnie · 2023-08-02T11:05:09Z

Closes #865
Closes #1417
Partially addresses #952

This is @AaronDCole's #1175 squash rebased onto master plus some tidying up and optimisation and fixes for a couple of small issues.

Update:

I've split the GScan part of the TreeItem component into its own component to address the performance concerns of TreeItem component: computed properties that are only relevant for GScan #1417
I went on to actually flatten single-child workflow-part nodes at the time of sorting them

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).
Tests are included
CHANGES.md entry included if this is a change that can affect users
No docs
If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

wxtim · 2023-08-03T06:52:59Z

Review

Initial testing looks good

Is it OK (probably tangential) that the task state LEDs dissapear under the edge of the GScan if the sidebar isn't wide enough? The do reappear if you resize the bar. I have not yet checked this on master, but will do so once I've finished this review and am happy to put the branch I'm reviewing away.

Possible Bug

If I change

# global.cylc
[install]
  max depth = 8

Workflows installed at a depth > 4 don't show at all!

Wheras on master:

CHANGES.md

MetRonnie · 2023-08-03T09:53:02Z

If I change
# global.cylc
[install]
  max depth = 8
Workflows installed at a depth > 4 don't show at all!

They do for me

src/components/cylc/tree/TreeItem.vue

Co-authored-by: Aaron David Cole <[email protected]>

Flatten them instead of automatically collapsing them

oliver-sanders

Code looks good (was a fun diff to unravel).

Workflow collapsing looks good, haven't tested deeply yet, will give that a go shortly.

I did spot one issue where the workflow icons stopped being clickable after state change.

Also, another one, but I'm not sure if it's related to this PR. The counter doesn't make a whole lot of sense to me:

This is made more pronounced by the collated totals for a workflow installation hierarchy as the counters don't appear to be summed?

A possible enhancement to the state total summing which would be really handy if it's easy to squeeze into this PR. Keep the collapsed summary open when the node is expanded. I.E. always show the aggregate state totals.

src/components/cylc/tree/GScanTreeItem.vue

oliver-sanders · 2023-10-23T10:46:33Z

src/components/cylc/tree/TreeItem.vue

-            :startTime="(latestJob(node) || {}).startedTime"
+      <slot v-bind="{ isExpanded }">
+        <!-- the node value -->
+        <!-- TODO: revisit these node.type values that can be replaced by constants later (and in other components too). -->


Any idea what this TODO means?

I think it means

-v-if="node.type === 'cycle'" +v-if="node.type === CYCLE"

oliver-sanders · 2023-10-23T10:47:25Z

src/components/cylc/tree/TreeItem.vue

+        <div
+          :class="nodeDataClass"
+          :style="nodeDataStyle"
+        >
+          <template v-if="node.type === 'cycle'">
+            <!-- NOTE: cycle point nodes don't have any data associated with them
+              at present so we must use the root family node for the task icon.
+              We don't use this for the v-cylc-object as that would set the node
+              type to family. -->
+            <Task
+              v-cylc-object="node"
+              v-if="node.familyTree?.length"
+              :key="node.id"
+              :task="node.familyTree[0].node"


Should the TreeView specific code go into a TreeViewTreeItem component similar to how the GScanView specific code when into GScanTreeItem component?

I thought about it but decided to hold off. Performance-wise, I don't think this is too much of a problem because there is not likely to be too many workflows in GScan compared to cycle points/families/tasks in the Tree view which could be hundreds. (Having said that, we did see that one case of run249!)

So, it probably should go there, but there's no pressure to do so ATM?

Want me to open an issue for it?

Can do, I was mostly just trying to understand the motive for the changes.

src/components/cylc/tree/TreeItem.vue

oliver-sanders · 2023-10-23T11:54:26Z

tests/e2e/specs/graphiql.cy.js

@@ -16,7 +16,7 @@
 */

 /** query used for the graphiql test */
-const query = `query App {
+const query = `query Workflow {


I don't know how this works but when you run this query in the Cypress window you get the response from the field with the matching name at src/services/mock/json/index.cjs. (But you get an empty response if running the query in the normal browser 🤷‍♂️.) Because I changed App to be an array of workflows, the result of this query seems to be the last workflow in the array. But I didn't change Workflow so it still returns workflow one

MetRonnie · 2023-10-24T10:17:38Z

The counter doesn't make a whole lot of sense to me:

It means 0 tasks currently in the submitted state. But lists tasks that were recently submitted.

MetRonnie · 2023-10-24T11:28:09Z

Is it OK (probably tangential) that the task state LEDs dissapear under the edge of the GScan if the sidebar isn't wide enough? The do reappear if you resize the bar.

Fixed btw (in 4404fc0 I think)

oliver-sanders · 2023-10-24T14:18:50Z

MetRonnie#6

oliver-sanders · 2023-10-24T14:21:53Z

Nice!

…ves a child of many-child workflow-parts

MetRonnie · 2023-10-24T16:27:27Z

MetRonnie#6

I think this is the case you are talking about - a single-child workflow-part that is itself a child of a many-child workflow-part?

Fixed in 90c92dd

Binding `:class` on the component that the `v-cylc-object` directive is applied to interfered with adding the `c-interactive` class in the directive. So I have changed it to a data attribute.

oliver-sanders

🚀

wxtim

Tried to break locally.
Read the code.

MetRonnie added the UX/UI User experience and interface improvements label Aug 2, 2023

MetRonnie added this to the 2.1.0 milestone Aug 2, 2023

MetRonnie requested review from oliver-sanders and wxtim August 2, 2023 11:05

MetRonnie assigned AaronDCole and MetRonnie Aug 2, 2023

MetRonnie changed the title ~~Auto-collapsing GScan~~ Auto-collapsing GScan nodes Aug 2, 2023

MetRonnie force-pushed the gscan branch from 4151ebf to 58a08f3 Compare August 2, 2023 11:14

MetRonnie mentioned this pull request Aug 2, 2023

TreeItem component: computed properties that are only relevant for GScan #1417

Closed

wxtim reviewed Aug 3, 2023

View reviewed changes

CHANGES.md Show resolved Hide resolved

oliver-sanders reviewed Aug 7, 2023

View reviewed changes

src/components/cylc/tree/TreeItem.vue Outdated Show resolved Hide resolved

MetRonnie marked this pull request as draft August 10, 2023 15:18

oliver-sanders modified the milestones: 2.1.0, 2.2.0 Aug 15, 2023

MetRonnie force-pushed the gscan branch from 58a08f3 to 4deb598 Compare August 25, 2023 15:10

MetRonnie force-pushed the gscan branch from 4deb598 to 0817fda Compare September 8, 2023 12:27

MetRonnie changed the title ~~Auto-collapsing GScan nodes~~ Auto-flatten single-child directories in GScan sidebar Oct 9, 2023

AaronDCole and others added 11 commits October 12, 2023 10:12

Collapsing GScan

ab9f7d3

Co-authored-by: Aaron David Cole <[email protected]>

Tidy & small optimizations

ef723c9

Fix expand/collapse of GScan tree items

acf3070

Offline mode: add hierarchical workflows to mock data

1db1e69

TreeItem component: split job details into separate component

50f840c

TreeItem component: split GScanTreeItem into separate component

999bdfe

TreeItem: do not render children until first expanded, for efficiency

d804f9c

GScan tree item: flatten 1-child workflow-parts when sorting nodes

86df4d9

Flatten them instead of automatically collapsing them

GScan: show ID tooltip for workflow-parts

4404fc0

TreeItem: remove unused click handling

e6bf645

Update changelog

53c18bc

MetRonnie force-pushed the gscan branch from 0817fda to 53c18bc Compare October 12, 2023 16:10

MetRonnie marked this pull request as ready for review October 12, 2023 16:12

MetRonnie requested review from oliver-sanders and wxtim October 17, 2023 16:35

MetRonnie mentioned this pull request Oct 18, 2023

Improve workflows in the offline/testing mode #952

Open

2 tasks

Merge branch 'master' into gscan

695dfb6

oliver-sanders reviewed Oct 23, 2023

View reviewed changes

MetRonnie added 2 commits October 24, 2023 12:25

Tidy GScanTreeItem code

4628499

GScan: always show aggregate state totals

520b945

MetRonnie requested a review from oliver-sanders October 24, 2023 11:31

GScan: fix flattening of single-child workflow-parts that are themsel…

90c92dd

…ves a child of many-child workflow-parts

MetRonnie added 2 commits October 24, 2023 18:03

Fix clicking on workflow icon to open mutation menu

2febe75

Binding `:class` on the component that the `v-cylc-object` directive is applied to interfered with adding the `c-interactive` class in the directive. So I have changed it to a data attribute.

Fix type error

d75ca04

oliver-sanders approved these changes Oct 25, 2023

View reviewed changes

wxtim approved these changes Oct 26, 2023

View reviewed changes

wxtim merged commit 0a4450b into cylc:master Oct 26, 2023
8 checks passed

MetRonnie deleted the gscan branch October 26, 2023 08:57

MetRonnie mentioned this pull request Nov 9, 2023

Fix GScan bug when filtering hierarchical workflows by state #1549

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-flatten single-child directories in GScan sidebar #1416

Auto-flatten single-child directories in GScan sidebar #1416

MetRonnie commented Aug 2, 2023 •

edited

Loading

wxtim commented Aug 3, 2023 •

edited

Loading

MetRonnie commented Aug 3, 2023

oliver-sanders left a comment

oliver-sanders Oct 23, 2023

MetRonnie Oct 23, 2023

oliver-sanders Oct 23, 2023

MetRonnie Oct 23, 2023

oliver-sanders Oct 23, 2023

MetRonnie Oct 23, 2023

oliver-sanders Oct 24, 2023

oliver-sanders Oct 23, 2023

MetRonnie Oct 23, 2023

MetRonnie commented Oct 24, 2023

MetRonnie commented Oct 24, 2023 •

edited

Loading

oliver-sanders commented Oct 24, 2023

oliver-sanders commented Oct 24, 2023 •

edited

Loading

MetRonnie commented Oct 24, 2023

oliver-sanders left a comment

wxtim left a comment

Auto-flatten single-child directories in GScan sidebar #1416

Auto-flatten single-child directories in GScan sidebar #1416

Conversation

MetRonnie commented Aug 2, 2023 • edited Loading

Check List

wxtim commented Aug 3, 2023 • edited Loading

Review

Possible Bug

MetRonnie commented Aug 3, 2023

oliver-sanders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MetRonnie commented Oct 24, 2023

MetRonnie commented Oct 24, 2023 • edited Loading

oliver-sanders commented Oct 24, 2023

oliver-sanders commented Oct 24, 2023 • edited Loading

MetRonnie commented Oct 24, 2023

oliver-sanders left a comment

Choose a reason for hiding this comment

wxtim left a comment

Choose a reason for hiding this comment

MetRonnie commented Aug 2, 2023 •

edited

Loading

wxtim commented Aug 3, 2023 •

edited

Loading

MetRonnie commented Oct 24, 2023 •

edited

Loading

oliver-sanders commented Oct 24, 2023 •

edited

Loading