Skip to content

feat(gfi): migrate plugin#447

Open
oeninghe-dataport wants to merge 33 commits intonextfrom
vue3/migrate-plugin-gfi
Open

feat(gfi): migrate plugin#447
oeninghe-dataport wants to merge 33 commits intonextfrom
vue3/migrate-plugin-gfi

Conversation

@oeninghe-dataport
Copy link
Copy Markdown
Collaborator

Summary

Migrate the GFI plugin.

Instructions for local reproduction and review

  • Open snowbox.
  • Click on a marker.
  • See the result.

Additional hints

  • The utils requestGfi* were migrated as-is and do not need to be reviewed therefore.

Relevant tickets, issues, et cetera

Closes #368

@oeninghe-dataport oeninghe-dataport added this to the POLAR@3 milestone Jan 8, 2026
@oeninghe-dataport oeninghe-dataport self-assigned this Jan 8, 2026
@oeninghe-dataport oeninghe-dataport added the refactor Refactoring of previous code label Jan 8, 2026
@oeninghe-dataport oeninghe-dataport linked an issue Jan 12, 2026 that may be closed by this pull request
@oeninghe-dataport oeninghe-dataport force-pushed the vue3/migrate-plugin-gfi branch 3 times, most recently from 58e73f6 to f941649 Compare January 16, 2026 13:12
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 23, 2026

PR Preview Action v1.8.0

QR code for preview link

🚀 View preview at
https://Dataport.github.io/polar/pr-preview/pr-447/

Built to branch gh-pages at 2026-04-21 16:50 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@oeninghe-dataport oeninghe-dataport marked this pull request as ready for review February 17, 2026 16:02
@dopenguin dopenguin removed the request for review from warm-coolguy February 19, 2026 09:46
# Conflicts:
#	examples/snowbox/services.js
Copy link
Copy Markdown
Member

@dopenguin dopenguin left a comment

Choose a reason for hiding this comment

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

  • Please add an example to iceberg; this maybe should include an example with a layer where the feature list is not being used
  • There should be no horizontal scrollbar if no features are available Image
  • Some things are missing / quite different with the featureList; some parts are connections with the markers feature. This includes:
    • When hovering an element in the feature list, the feature is highlighted in the map with the hover style
    • When hovering an element in the map, the feature is highlighted in the feature list (previously green); when hovering a clustered feature, all features that are part of the cluster are highlighted
    • If I select a feature in the map, it is selected in the feature list
    • If I select a feature in the feature list, the corresponding marker gets the selected style; currently, a yellow dot is being displayed
    • If I select a feature in the feature list, the map should be centered on that feature
    • If a feature is not selectable because of the configured isSelectable function, it is not being shown in the feature list

The list may not be complete, so please take a look at Meldemichel regarding the various things mentioned above.

I'll be taking a look at the components and stores once you've tackled these things.

🏓 @oeninghe-dataport

Comment thread src/plugins/gfi/utils/requestGfiWfs.ts Outdated
Comment thread src/plugins/gfi/utils/requestGfiWfs.ts
Comment thread src/plugins/gfi/utils/requestGfiWms.ts Outdated
const features: Feature[] = []
let feature: Feature | undefined

/* TODO: Format supposedly looks like this – is this a standard or arbitrary?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we able to answer this? Seems like a good point to maybe get rid of this comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reading the current WMS spec, I could not find this format. However, this format may be specified in another way; or may at least be default for some implementations.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unfortunate! Please update the comment with your acquired knowledge.

Comment thread src/core/stores/main.ts
Comment on lines +83 to +86
function getLayer(layerId: string) {
return map.value.getAllLayers().find((layer) => layer.get('id') === layerId)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are 4 instances where we currently use coreStore.map.getLayers().getArray().find(...) where this action could also be used.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Those 4 places are utils, and utils should not use stores (this would make unit tests harder, although we don't write them at the moment).

I also know that this rule is already violated in some utils, but I don't want to introduce more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then that rather sounds like this action should become a utility function so it can be used across the project.

Comment thread src/plugins/geoLocation/types.ts
Comment thread examples/snowbox/index.js Outdated
Comment thread src/locales.ts Outdated
Comment thread src/locales.ts Outdated
Comment thread src/locales.ts Outdated
dopenguin and others added 5 commits February 23, 2026 19:56
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
Co-authored-by: Pascal Röhling <73653210+dopenguin@users.noreply.github.com>
@dopenguin
Copy link
Copy Markdown
Member

@oeninghe-dataport pls @ me once you've tackled all the things!

@oeninghe-dataport
Copy link
Copy Markdown
Collaborator Author

oeninghe-dataport commented Apr 16, 2026

  • When hovering an element in the feature list, the feature is highlighted in the map with the hover style
  • When hovering an element in the map, the feature is highlighted in the feature list (previously green); when hovering a clustered feature, all features that are part of the cluster are highlighted

For the hovering problems, this should be fixed with 9f25413.
As your expected behaviour depends on bindWithCoreHoverSelect, I enabled this for the snowbox w/ 4dfe6af.

@oeninghe-dataport
Copy link
Copy Markdown
Collaborator Author

  • There should be no horizontal scrollbar if no features are available Image

875a35c

@oeninghe-dataport
Copy link
Copy Markdown
Collaborator Author

  • If I select a feature in the map, it is selected in the feature list
  • If I select a feature in the feature list, the corresponding marker gets the selected style; currently, a yellow dot is being displayed
  • If I select a feature in the feature list, the map should be centered on that feature

ec689f1, a6e5698

@oeninghe-dataport
Copy link
Copy Markdown
Collaborator Author

  • Please add an example to iceberg; this maybe should include an example with a layer where the feature list is not being used

3271a30

Feature list is not used, and you can choose between directSelect, directSelect + multiSelect, and coordinateSource from pins plugin.

@oeninghe-dataport
Copy link
Copy Markdown
Collaborator Author

I think I've addressed all the points now.

🏓 @dopenguin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactoring of previous code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migration of GFI plugin to POLAR@3

2 participants