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

Adds a layer to display location-based groups #102

Merged
merged 3 commits into from
Aug 20, 2022
Merged

Adds a layer to display location-based groups #102

merged 3 commits into from
Aug 20, 2022

Conversation

rafmudaf
Copy link
Contributor

@rafmudaf rafmudaf commented Aug 9, 2022

This pull request adds a layer to the map to display RSE groups tied to a location (city or region) as described in USRSE/usrse.github.io#876.

Screen Shot 2022-08-09 at 3 21 33 PM

@cosden
Copy link
Member

cosden commented Aug 20, 2022

@rafmudaf Looks great! I just pulled it down and previewed locally. It looks slick and I love that clicking on the regional group brings you straight to the slack channel. The colors and marker size all look great. Nice job! Sorry it took so long for me to get to it.

I'm been trying to think of any issues, and I have one potential concern that maybe we can brainstorm together as to how to resolve it. In presentations I often show the membership map to visually show "where our members are located" and the change over time. It would be great to have the ability to toggle the regional groups on and off to be consistent. Outside of that (which seems like a lot of work), should we just have a separate page that only insiders know where it is?
Any other ideas?

@danielskatz
Copy link
Contributor

maybe I shouldn't have merged the post PR without this one...

@danielskatz
Copy link
Contributor

@cosden - I think we should merge this now, then address your issue above following that. If you agree, maybe go ahead and merge it

@cosden
Copy link
Member

cosden commented Aug 20, 2022

Yes, I agree. Let's merge. We can worry about this later. I'll open an issue to keep track of it. Honestly, it might only be me that cares about it, and I'm happy to have a page that isn't linked from anywhere that does this.

@cosden cosden merged commit 85cf911 into USRSE:master Aug 20, 2022
@rafmudaf
Copy link
Contributor Author

rafmudaf commented Sep 7, 2022

@cosden I fully agree with your concern and I had also thought about this. It was just a perfect-vs-good situation. I'll think about some solutions to this, and feel free to assign me to #103 if its helpful.

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.

4 participants