Skip to content

Simplify app toolbar - #2557

Open
MetRonnie wants to merge 5 commits into
cylc:masterfrom
MetRonnie:app-toolbar
Open

Simplify app toolbar#2557
MetRonnie wants to merge 5 commits into
cylc:masterfrom
MetRonnie:app-toolbar

Conversation

@MetRonnie

@MetRonnie MetRonnie commented Jun 9, 2026

Copy link
Copy Markdown
Member

In #2542 we were not 100% agreed on the new icon for the nav sidebar. I am proposing this icon for when the sidebar is open:

image

and this for when it closed:

image

This PR also includes a few changes to eliminate technical debt that I had gradually accumulated to the point where I think we should get them in.

  • Removing the app module from the vuex store as all it is used for is setting the toolbar title and there are simpler ways to do this
  • Replacing the GraphQL mixin with a composable and simplifying data flow across the app by eliminating some props

Users coming from Vue 2 may be familiar with the mixins option, which also allows us to extract component logic into reusable units. There are three primary drawbacks to mixins:

  1. Unclear source of properties: when using many mixins, it becomes unclear which instance property is injected by which mixin, making it difficult to trace the implementation and understand the component's behavior.
  2. Namespace collisions: multiple mixins from different authors can potentially register the same property keys, causing namespace collisions.
  3. Implicit cross-mixin communication: multiple mixins that need to interact with one another have to rely on shared property keys, making them implicitly coupled.

For the above reasons, we no longer recommend using mixins in Vue 3. The feature is kept only for migration and familiarity reasons.

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).
  • No changelog entry needed as invisible to users

@MetRonnie MetRonnie added this to the 2.15.0 milestone Jun 9, 2026
@MetRonnie MetRonnie self-assigned this Jun 9, 2026

@ChrisPaulBennett ChrisPaulBennett left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is still an inconsistency between workflowID vs workflowId.
I think something is changing the case (GraphQL maybe?)
This may not actually be an issue, just thought I'd bring it up since you have altered a few of them.
Much better icons!

@MetRonnie

Copy link
Copy Markdown
Member Author

I was hesitant to change the GraphQL variable $workflowId but actually the variable name doesn't matter - it won't break anything. Changed to $workflowID now

@MetRonnie

Copy link
Copy Markdown
Member Author

Ping @samuel-denton are you happy with these icons for the sidebar toggle button?

@samuel-denton

Copy link
Copy Markdown
Contributor

Ping @samuel-denton are you happy with these icons for the sidebar toggle button?

Yep I like those, consistent with everything else.

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.

3 participants