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

Anchor plugin adds &p=anchor #1294

Closed
jesade-vbg opened this issue Mar 1, 2023 · 5 comments
Closed

Anchor plugin adds &p=anchor #1294

jesade-vbg opened this issue Mar 1, 2023 · 5 comments
Assignees
Labels
Milestone

Comments

@jesade-vbg
Copy link
Contributor

The Achor plugin should exclude itself when generating url.
&p=anchor

Issue appeares to be related to #1252

@jesade-vbg jesade-vbg added the bug label Mar 1, 2023
@jacobwod jacobwod changed the title Achor plugin adds &p=anchor Anchor plugin adds &p=anchor Mar 1, 2023
@jacobwod
Copy link
Member

jacobwod commented Mar 1, 2023

Sure, I can fix that. The implication will be that there will be no way to tell Hajk to start with Anchor plugin visible. But perhaps that's never intended anyway. Just wanted to have it said, for the record.

@jacobwod
Copy link
Member

jacobwod commented Mar 1, 2023

I tried with a basic check that excludes Anchor from the p parameter, like this:

#getVisiblePlugins = () =>
    this.#app.windows
      .filter((w) => w.state.windowVisible && w.type !== "anchor")
      .map((p) => p.type)
      .join();

The result is no good:
https://user-images.githubusercontent.com/110222/222101668-e1f8ea4b-2079-4e0c-933e-af6419d6cc7c.mov

Reason for this is:

  1. User has a plugin visible, e.g. Measurer and clicks on Anchor to show it instead
  2. This triggers visibilityChanged for plugins
  3. When visiblityChanged is trigger for plugins, we ensure that the generated hash is updated (after all, that's how this mechanism works: user clicks on LayerSwitcher, its visibility changes and it is added to the p param)
  4. So clicking on Anchor triggers a anchorModel.getAnchor(). We're in.
  5. On thing that getAnchor() does is #getVisiblePlugins(). You see the code above. Note that we exclude anchor on purpose.
  6. When getAnchor() is done, with the new string that now does not contain anchor, we set browser's hash string to the newly retrieved value.
  7. This triggers an onHashChanged event.
  8. We go into the listener for onHashChanged. On of the things we do there is to ensure that correct plugins are visible. This is done by comparing currently visible windows with the value of p.
  9. Since anchor is not a part of p, we send a closeWindow to the Anchor plugin, which closes itself.
  10. Boom.

@jesade-vbg
Copy link
Contributor Author

Hmm ok. I understand. But we dont even use #1252 (yet).
Maybe anchor should be excluded from closeWindow as well? Just leave it be.

I know it's a bit hacky but to share the share-plugin (anchor) seems a bit odd.

@jacobwod
Copy link
Member

jacobwod commented Mar 1, 2023

Yeah, it's not supposed to be included. I thought about telling the Anchor plugin ignore whether it's in the p-parameter or not. That could work. We'll end up with p=anchor anyway, which isn't nice, but at least it will not show itself.

@jesade-vbg
Copy link
Contributor Author

jesade-vbg commented Mar 1, 2023

You're fix is working good when I'm testing! Its very clear in the code how it works, constant and all. Good work, thanx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants