mmomtchev / rlayers

React Component Library for OpenLayers
ISC License
173 stars 36 forks source link

Add select interaction #277

Open ludvigeriksson opened 3 months ago

ludvigeriksson commented 3 months ago

This pull request adds the select interaction to rlayers.

Following the React spirit described in the readme, I opted to automatically add a layer filter if the interaction is placed inside a vector layer. See examples below. Do you agree with this decision?

In this example only features from the vector layer of which the interaction is placed inside will be selectable.

<RMap>
    <RLayerVector>
        <RSelect onSelect={(event) => console.log(event)} />
    </RLayerVector>
    <RLayerVector />
</RMap>

In this example features from every vector layer will be selectable.

<RMap>
    <RSelect onSelect={(event) => console.log(event)} />
    <RLayerVector />
    <RLayerVector />
</RMap>

I did not find any documentation for contributing, so I'm not sure how examples and tests are set up. If I can get some pointers for this I can try to add these as well.

mmomtchev commented 3 months ago

As you may have seen from my Github profile, I am currently unemployed and living on social welfare because of a huge extortion involving the French judiciary and the French police. The object of this extortion is that I accept a job in a company where I will be getting various viagra calls and dicks messages simultaneous with orders from my supervisor - in order to ease the pain of a guy with a serious issue. In order to intimidate me, I am getting simultaneous posts on Github - this PR is simultaneous with a comment on a discussion in the Vercel repository where I frequently get such comments.

Thus said, if you add a proper unit test and fix the failing ones, this will get merged, as making good software is the main reason I am here on Github.

However I insist to also be able to note those occurrences.

ludvigeriksson commented 3 months ago

Hello Momtchil, and thank you for a quick answer.

I'm sorry to hear about your situation. I must say I am a bit discouraged to contribute to this repository when being met with this tone, as well as the response to the bug report issue I created earlier yesterday. However, if this happened to coincide with other comments you received, I'm sorry about that coincidence, and I hope we can keep it professional going forwards.

I will look at the existing unit tests and match them as well as I can for the RSelect interaction. However, the currently failing unit test seems to be for the RLayerStadia component, which fails on the main branch as well.

I would also like to take this opportunity to say kudos for a very nice implementation of OpenLayers for React. I'm currently working at Vidheim (https://www.vidheim.com, in Swedish only, sorry), which is a newly established company that sells maps and services mostly within the forestry industry, but I've worked in the forestry/GIS industry for over 6 years. During this time I have (actually twice) implemented a very similar (closed source) library, but it never reached quite the coverage that this library has. I especially like how smooth the inheritance worked when implementing a new component, for example the automatic event handlers.

ludvigeriksson commented 2 months ago

Do you need me to add anything more or are my tests sufficient for getting this merged?

mmomtchev commented 2 months ago

@ludvigeriksson can you rebase against the latest main?