Skip to content

Conversation

harleenkaur2003
Copy link
Contributor

@harleenkaur2003 harleenkaur2003 commented May 1, 2025

Title:

Display Latest Job Platform on Unexpanded Task Node

Description:

  • This pull request addresses issue Tree view: show latest job platform next to unexpanded task #2153.
  • Displays the latest job’s platform (e.g., [localhost]) next to collapsed task nodes in the Tree View.
  • Mimics the behavior of the job summary icon — platform info disappears once the node is expanded.
  • Uses the existing platform property for consistency with job nodes.
  • Ensures UI alignment with the provided mockup.

Screenshot:
Screenshot (345)

Checklist:

  • 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). [Tests are not included since this is a visual UI change with no backend logic or new components]
  • Changelog entry not included - UI is improved but this is a minor visual enhancement
  • Docs not required- this change does not require documentation update

@hjoliver
Copy link
Member

hjoliver commented May 1, 2025

Thanks @harleenkaur2003! We'll take a look at this soon.

@MetRonnie MetRonnie added this to the 2.x milestone May 7, 2025
@MetRonnie MetRonnie linked an issue May 7, 2025 that may be closed by this pull request
@harleenkaur2003
Copy link
Contributor Author

Hey @hjoliver @MetRonnie !
Just checking in to see if there's any feedback needed on this pr. I'd be happy to make any changes if required.
Thanks!

@hjoliver
Copy link
Member

Sorry for the delay @harleenkaur2003 - we are just slammed with higher priority issues at the moment. However, this is quite a small code change so maybe we can take a look very soon...

@oliver-sanders oliver-sanders requested a review from hjoliver May 20, 2025 12:39
@hjoliver
Copy link
Member

hjoliver commented May 21, 2025

This looks good to me.

Tested with a platform group, of two faked platforms:

# global.cylc
[platforms]
    [[one]]
        hosts = localhost
        install target = localhost
    [[two]]
        hosts = localhost
        install target = localhost
[platform groups]
    [[nums]]
        platforms = one, two

image

@cylc/core -

  • the code looks good to me, but need a second review from someone more familiar with the UI codebase
    • I've added @MetRonnie, as author of the associated issue (however, I think he is on leave at the moment)
  • note the platform name, when collapsed, uses a smaller font and square brackets, which differs from the mock-up on the issue. Question: this might be sensible as it correctly suggests that the name is more loosely associated with the task (than with the job, when expanded), but on the other hand it looks a bit busier - preference?

@hjoliver hjoliver requested a review from MetRonnie May 21, 2025 03:29
@harleenkaur2003
Copy link
Contributor Author

Thank you for the review and feedback!

Regarding the platform name style, I used brackets and a smaller font to visually separate the platform from the task name, but I'm happy to align it with mock-up style if preferred - just let me know!

Looking forward to @MetRonnie's review.

Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Hi @harleenkaur2003, thanks for your contribution. I've added some suggestions

Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution 👍

@MetRonnie MetRonnie requested a review from hjoliver June 9, 2025 16:02
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

(Waiting for approval from @hjoliver )

@harleenkaur2003
Copy link
Contributor Author

Hey @hjoliver !
Just following up to see if the PR is ready for your review .Please let me know if any additional changes are needed from my side.
Thankyou.

@hjoliver
Copy link
Member

hjoliver commented Jul 2, 2025

Apologies @harleenkaur2003 - it just slipped off my radar screen! Taking a look today...

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.

Great, thanks @harleenkaur2003 🎉

@hjoliver hjoliver merged commit 744f384 into cylc:master Jul 2, 2025
8 checks passed
@MetRonnie MetRonnie modified the milestones: 2.x, 2.8.0 Jul 31, 2025
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.

Tree view: show latest job platform next to unexpanded task

3 participants