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

New: Change the API to handle OrbView and OrbMapView #34

Merged
merged 3 commits into from
Nov 9, 2022

Conversation

tonilastre
Copy link
Contributor

@tonilastre tonilastre commented Nov 7, 2022

The following PR will include API changes to the views such as:

  • There are separate classes for orb default view (OrbView) and orb map view (OrbMapView)
  • It fixes the Typescript issue with settings: Fix the Typescript usage of the view settings #16
  • Orb view (OrbView or OrbMapView) is the main point where it receives a container and optional settings, data, and events are part of the orb view
  • When changing the views on the same container, view.destroy() should be called on the previous view to remove event listeners and content of the container
  • Hover and click strategy changed into view parameters (strategy.isDefaultSelectEnabled, strategy.isDefaultHoverEnabled) with an example in docs/...md on how to overwrite with custom select/hover strategy
  • Fixed the issue when the strategy was missing, click/hover were not working

@tonilastre tonilastre requested a review from cizl as a code owner November 7, 2022 12:33
@tonilastre tonilastre marked this pull request as draft November 7, 2022 12:33
@tonilastre tonilastre marked this pull request as ready for review November 7, 2022 17:04
@tonilastre tonilastre self-assigned this Nov 7, 2022
Copy link
Contributor

@cizl cizl left a comment

Choose a reason for hiding this comment

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

The API even looks cleaner now, and I like the new strategy handling.

It seems to me that are starting to have a couple of reused pieces of markdown in our docs (the strategy section for example). This might get difficult to maintain down the line. Just thinking out loud: maybe we could introduce pandoc or some other preprocessor to build our final docs. (for example reference and reuse the strategy section in multiple files)

@tonilastre
Copy link
Contributor Author

The API even looks cleaner now, and I like the new strategy handling.

It seems to me that are starting to have a couple of reused pieces of markdown in our docs (the strategy section for example). This might get difficult to maintain down the line. Just thinking out loud: maybe we could introduce pandoc or some other preprocessor to build our final docs. (for example reference and reuse the strategy section in multiple files)

You are 100% right. We can touch base on the docs after you are done with the simulator part. Skip the docs changes for it so we can figure out how to handle it. How does that sound to you?

@tonilastre tonilastre changed the base branch from main to release/0.2.x November 8, 2022 14:59
@tonilastre tonilastre merged commit ab9ba31 into release/0.2.x Nov 9, 2022
@tonilastre tonilastre deleted the new-change-view-api branch November 9, 2022 16:23
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.

None yet

2 participants