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.

Sign up for GitHub

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

Jump to bottom

Show flow numbers #2016

Merged
merged 3 commits into from
Jan 8, 2025
Merged

Show flow numbers #2016

merged 3 commits into from
Jan 8, 2025

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented Dec 6, 2024 •
edited
Loading

Partially addresses #468

Flow numbers that are not 1 or None are now shown in the tree and table views. Tasks in flow=None are dimmed in the tree, table and graph views.

image

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 (or explain why tests are not needed).
  • Changelog entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.

@MetRonnie MetRonnie added this to the 2.8.0 milestone Dec 6, 2024
@MetRonnie MetRonnie self-assigned this Dec 6, 2024
@MetRonnie MetRonnie changed the title Show flow numbers in tree view & command menu Show flow numbers Dec 31, 2024
@MetRonnie MetRonnie mentioned this pull request Jan 3, 2025
Closed
@oliver-sanders oliver-sanders modified the milestones: 2.8.0, 2.7.0 Jan 3, 2025
@oliver-sanders
Copy link
Member

(optimistically bumped to 2.7.0 while other work is ongoing)

@oliver-sanders
Copy link
Member

Partially addresses #468

I think this may as well close #468, the visual filtering suggestions have been carried on in #470

MetRonnie
MetRonnie commented Jan 3, 2025
View reviewed changes
src/utils/tasks.js Outdated Show resolved Hide resolved
@MetRonnie MetRonnie marked this pull request as ready for review January 6, 2025 18:24
@oliver-sanders oliver-sanders mentioned this pull request Jan 7, 2025
Open
oliver-sanders
oliver-sanders approved these changes Jan 7, 2025
View reviewed changes
cypress/component/treeItem.cy.js Outdated Show resolved Hide resolved
@MetRonnie
Simplify TreeItem and fix tests
c6ae6ae
@MetRonnie
Show flow nums
6de9d53 
Show flow nums in tree/table views if not 1 or None.
Dim flow=None in tree/table/graph views.
Show all flow nums in command menu.
Also update mock data.
@MetRonnie
Changelog
b5d310f
@hjoliver
Copy link
Member

hjoliver commented Jan 8, 2025

I borked testing this today by checking out MetRonnie/cylc-ui/flow-nums instead of cylc/cylc-ui/flow-nums - results were confusing as hell! ... is there some reason you've used a branch in the central repo?

hjoliver
hjoliver approved these changes Jan 8, 2025
View reviewed changes
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.

Having got the right branch 🤦 - this is great. Mainly a functional review, but the code looks fine to me and Oliver has approved ... merging.

@hjoliver hjoliver merged commit e263a14 into master Jan 8, 2025
7 checks passed
@hjoliver hjoliver deleted the flow-nums branch January 8, 2025 02:08
@hjoliver hjoliver mentioned this pull request Jan 8, 2025
Closed
4 tasks
@MetRonnie
Copy link
Member Author

I borked testing this today by checking out MetRonnie/cylc-ui/flow-nums instead of cylc/cylc-ui/flow-nums - results were confusing as hell! ... is there some reason you've used a branch in the central repo?

Sorry, reason was this

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliver-sanders oliver-sanders oliver-sanders approved these changes

@hjoliver hjoliver hjoliver approved these changes

Assignees

@MetRonnie MetRonnie

Labels
None yet
Projects
None yet
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@MetRonnie @oliver-sanders @hjoliver