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

feature: Add infowindow component to origo.api() #2075

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jokd
Copy link
Contributor

@jokd jokd commented Nov 8, 2024

Fixes #2074

@johnnyblasta
Copy link
Collaborator

You have a conflict to resolve @jokd

@steff-o
Copy link
Contributor

steff-o commented Nov 19, 2024

Maybe a little late to the party as the Infowindow already exists, but before it is exposed i the api there may be some issues that have to be resolved. As on now, it only seems to be used in two places: edithandler and search and could easily be changed. When it has been exposed it will be harder to change anything.

  • What is the main difference between infowindow and modal? Why not use modal?
  • The name is not ideal, as it conflicts with the THE InfoWindow.
  • I'm not sure how to use it as search and edithandler use it differently
    • The onAdd-eventlistener is called when Component is added to a parent Component. The edithandler never does that so it is not called and render() is called manually
    • The onAdd-eventlistener attaches the render-event to a non-existing function "onRender", which is confusing but seems harmless as search works, which adds to its child list which triggers the add event and thus render() is not called manually.
    • It is not clear if this is deliberately to support these different usages. It's tricky because the infowindow injects itself to the main div, it does not have a Component as its parent in the DOM and thus it does not behave like a sub component, but it can on the other hand be instantiated in multiple instances so it does behave like a global resource either (like the spinner).
  • Why is it exported under Origo.component name? Other ui components are under Origo.ui.
    • Maybe because it is implemented in the source tree under components, but that looks like it is ment for parts of the application layout, not class like objects implementing the Component. Infowindow seems to be able to be instantiated several times and looks more like a ui component to me.

@jokd
Copy link
Contributor Author

jokd commented Nov 21, 2024

Yeah and to spice up things even more we have the infowindow setting in the config where infowindow is one option. Based on your comment a small rework of the component is maybe a good idea.

Infowindow has as far as I know other features than the modal, but things might have changed since I last used it.

  • It has no backdrop and can be present while using the map.
  • You can have it floating or aligned to the left.
  • There are methods for altering the content.

Maybe it should have been put in the ui folder along with the modal but then perhaps the logger should as well.

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.

Infowindow in Origo API
3 participants