Skip to content

Fix teleport to wrong instance when multiple analysis views open #2163

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented May 8, 2025

Fixes a bug not recorded in an issue.

Steps to reproduce

Open 2 analysis view tabs in the same workspace, then switch to the box plot or time series mode in the second one. You would find the sorting control does not appear here, but if you go back to the first analysis view tab it will be there instead.

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
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@MetRonnie MetRonnie added this to the 2.8.0 milestone May 8, 2025
@MetRonnie MetRonnie self-assigned this May 8, 2025
@MetRonnie MetRonnie added bug Something isn't working small labels May 8, 2025
Comment on lines -125 to +124
:sort-input-teleport-target="toolbar?.id"
:sort-input-teleport-target="toolbarRef"
Copy link
Member Author

Choose a reason for hiding this comment

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

Avoid clashing IDs by specifying the template ref (HTML element) itself as the teleport target

/** @type {import('vue').Ref<HTMLElement>} template ref */
const toolbar = ref(null)
const toolbarRef = useTemplateRef('toolbar')

Copy link
Member Author

Choose a reason for hiding this comment

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

This is somewhat tangential, but is a newer (clearer IMO) way of defining template refs in Vue 3.5

Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

LGTM, but haven't tested.

@MetRonnie MetRonnie force-pushed the analysis-toolbar branch from a604e6b to 66108d8 Compare May 8, 2025 11:49
@MetRonnie
Copy link
Member Author

Just added a test

@MetRonnie MetRonnie force-pushed the analysis-toolbar branch from 66108d8 to f3fb6a4 Compare May 8, 2025 11:51
@oliver-sanders oliver-sanders added the data workflows team Work pertaining to the analysis/gantt/etc views label May 13, 2025
@MetRonnie
Copy link
Member Author

Ping @JAllen42 are you able to test out that this fixes the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working data workflows team Work pertaining to the analysis/gantt/etc views small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants