Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to View filtering, and table display and functionality #861

Merged
merged 12 commits into from
Jan 27, 2022

Conversation

AaronDCole
Copy link
Contributor

@AaronDCole AaronDCole commented Nov 29, 2021

This merge request updates the design and functionality of the table and tree views. It includes;

  • removed the requirement to manually 'submit' filtering options in the filter bar
  • fixed the filter bar to the top of the Lumino container in the table view
  • added a new column with a dropdown toggle to the task rows (if they have current jobs)
  • modified the table data store to include information about the jobs relating to the current task (and add, update and remove as instructed via delta changes)
  • modified the table to list the task jobs' when the jobs dropdown toggle is applied
  • fixed the display of the 'layered' task status icon
  • added the job status icon and submission number in the job rows
  • updated the respective tests

These changes partially address #471
These changes:

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).
  • Already covered by existing tests.
  • Appropriate change log entry included.
  • No documentation update required.

@AaronDCole AaronDCole requested a review from hjoliver November 29, 2021 06:02
@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2021

Codecov Report

Merging #861 (480a8d5) into master (1dd6dd1) will decrease coverage by 0.60%.
The diff coverage is 66.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #861      +/-   ##
==========================================
- Coverage   91.24%   90.63%   -0.61%     
==========================================
  Files          92       92              
  Lines        1930     1986      +56     
  Branches      114      146      +32     
==========================================
+ Hits         1761     1800      +39     
- Misses        143      160      +17     
  Partials       26       26              
Flag Coverage Δ
e2e 76.68% <8.47%> (-1.87%) ⬇️
unittests 78.19% <20.33%> (-2.10%) ⬇️

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

Impacted Files Coverage Δ
src/components/cylc/Drawer.vue 75.00% <ø> (ø)
src/components/cylc/gscan/GScan.vue 93.75% <ø> (ø)
src/components/cylc/tree/Tree.vue 86.76% <0.00%> (+1.91%) ⬆️
src/views/Table.vue 100.00% <ø> (ø)
src/views/Tree.vue 100.00% <ø> (ø)
src/views/Workflow.vue 92.85% <ø> (ø)
src/components/cylc/table/Table.vue 77.27% <66.66%> (-22.73%) ⬇️
src/components/cylc/table/deltas.js 81.33% <75.00%> (-1.75%) ⬇️

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 1dd6dd1...480a8d5. Read the comment docs.

@AaronDCole
Copy link
Contributor Author

rebuild

@hjoliver
Copy link
Member

hjoliver commented Dec 6, 2021

Looks good @AaronDCole.

Since you make the filter bar sticky on the table view, and got rid of the Filter button on both views, could you also make it sticky on the tree view?

Also, column sort (via clicking on the heading) would be an uncontroversial thing to get working before the (yet-to-be-arranged) meeting on the table view.

@hjoliver
Copy link
Member

hjoliver commented Dec 6, 2021

Also, job expansion would probably be better on the left, near the task icons, rather than off to the far right.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed. Looks good, a couple of minor comments so far.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@MetRonnie
Copy link
Member

MetRonnie commented Jan 18, 2022

I've noticed when clicking the "x" in the text field to clear the filtering by task name, nothing happens / the view doesn't update.

Also the job colours aren't always correct in the task row - I'm seeing turquoise and orange for some reason Actually this is only the case for the mock data when you do yarn serve

@MetRonnie
Copy link
Member

A couple more issues:

  • Vertical scrollbars have gone missing from the Tree and Table views

  • Clicking on the column defaults to sorting in descending order, which is counter-intuitive. Also I think an arrow icon makes more sense than a chevron for indicating sorting (chevron is better suited for expand/collapse imho)

    image

But overall a nice improvement 👍

@wxtim wxtim removed their request for review January 18, 2022 16:11
@wxtim
Copy link
Member

wxtim commented Jan 18, 2022

I've removed myself as a reviewer, I got sidetracked into playing with Slurm Job Arrays.

… view.

Made the table view scroll while keeping the filter inputs at the top of the container.
Added expandable rows to show jobs relating to an active task.
Modified the table store to track jobs relaxing to task proxies.
…ch job row (but disabled for now). Fixed a small bug with pruning the jobs from the task proxies.
…caused by the onchange handler working inconsistently, so Ive changed it to an 'onkeyup' event
Disabled the default (and broken) data-table header
Made both the table and tree view full width within their containers
Made the tree scroll within the container with static header
@AaronDCole
Copy link
Contributor Author

I've noticed when clicking the "x" in the text field to clear the filtering by task name, nothing happens / the view doesn't update.

Also the job colours aren't always correct in the task row - I'm seeing turquoise and orange for some reason Actually this is only the case for the mock data when you do yarn serve

I cant actually reproduce this, and I 'think' the orange color might be intentional. The other issues were all valid and Ill push fixes to them in a few minutes.

Fixed issue where clearing filters wouldn't trigger data refresh.
Replaced the svg html markup with a reference to the materials icons.
Changed the default column sort direction.
Changed column sort icon to arrow instead of chevron.
Leave overflow on auto so scrollbars automatically appear if needed in tree/table views
@MetRonnie
Copy link
Member

MetRonnie commented Jan 27, 2022

Strange thing happened when testing this out. I was running a simple workflow

[scheduling]
    cycling mode = integer
    runahead limit = P3
    [[graph]]
        P1 = foo
[runtime]
    [[foo]]
        script = """
            sleep 2
            if (( $RANDOM % 2 )); then exit 1; else exit 0; fi
        """

and triggering */foo a few times using the UI when

Received a Table delta before the workflow initial data burst

image

While I got this error, meanwhile the failed tasks I was triggering succeeded and the workflow shut down, even though there is no final cycle point

INFO - Workflow shutting down - AUTOMATIC
INFO - DONE

I have been able to reproduce several times

@MetRonnie
Copy link
Member

MetRonnie commented Jan 27, 2022

Ok so

@MetRonnie MetRonnie merged commit f193445 into master Jan 27, 2022
@MetRonnie MetRonnie deleted the views-updates branch January 27, 2022 13:03
@MetRonnie MetRonnie modified the milestones: 1.x, 1.0 Jan 27, 2022
MetRonnie added a commit to MetRonnie/cylc-ui that referenced this pull request Dec 23, 2022
MetRonnie added a commit to MetRonnie/cylc-ui that referenced this pull request Dec 23, 2022
MetRonnie added a commit to MetRonnie/cylc-ui that referenced this pull request Jan 11, 2023
oliver-sanders pushed a commit that referenced this pull request Jan 12, 2023
* Allow circular-dependency-plugin to work on Windows
* Tidy
* Fix typo
* e2e test: bring back necessary wait
* Try to fix graphiql e2e test flakiness
* Revert some unwanted changes from #861
* Fix tree/table view alignment issues
* Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instant filtering (by task name/state) Workflow Table View component
5 participants