Skip to content
This repository has been archived by the owner on May 20, 2024. It is now read-only.

Scofield map update, add new features to handle the community problem #2714

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

scofielddd
Copy link

What does this PR do?

This PR introduces several improvements and bug fixes to the marker selection feature on the interactive map component. The updates include:

  • Enhanced logic for marker selection to ensure robust state management and event handling.

  • Refactoring the event emission to utilize update:selectedMarkers, streamlining the interaction between child and parent components.

  • Additional unit tests to cover new scenarios in marker selection and deselection.

Links to related issues

https://community.karrot.world/t/enlarged-map-of-a-place-does-not-keep-place-in-focus/1457

User story: A person in my group trying to see a place on the map could not see it very well, so they tried to enlarge it (on the button on the right corner of the map). But doing that shows the whole map of the city with all the places

The best would be to show the enlarged map still focusing on that specific place
Outcomes (or what needs to be done to move forward): implement, if no objections

Checklist

  • [Y] added a test, or explain why one is not needed/possible...
  • [Y] no unrelated changes
  • [Y] asked someone for a code review
  • [Y] joined #karrot:matrix.org
  • [N] added an entry to CHANGELOG.md (description, pull request link, username(s))
  • [Y] tried out on a mobile device (does everything work as expected on a smaller screen?)

@nicksellen
Copy link
Member

Hey, thanks for contributing, a few points:

Student contributions

We have very limited capacity for working with undergrad students - I'm not sure if you're read https://github.com/karrot-dev/karrot-frontend/blob/master/CONTRIBUTE.md or https://community.karrot.world/t/are-you-a-student-who-wants-to-contribute-in-karrot/1449 and followed that guidance first?

It is difficult for us to accept beginner-level code contributions as this always requires lots of support and help. We don't have capacity for that for likely-one-time contributors, so we prefer to focus on contributors that are interested in joining our team for the longer term.

I think we should make this clearer.

PR does not address issue

I tried out your branch, but it does not address the original issue:

A person in my group trying to see a place on the map could not see it very well, so they tried to enlarge it (on the button on the right corner of the map). But doing that shows the whole map of the city with all the places

The best would be to show the enlarged map still focusing on that specific place

here are the steps I took:

  1. I'm on the place page

image

  1. I click the enlarge button in the top right corner

image

I now have a view of ALL the places, but it is not focused on the specific place I was on.

I see you have implemented the ability to click a place once you are on the map, but that doesn't help the user as described in the original issue.

no unrelated changes

You checked the box that says you have made "no unrelated changes" but this is not true. You have added a travis yml file, and a random file you used to trigger builds?

You also have debugging logging still in the code.

I have not reviewed the code itself given these issues.

Also, to note, we have migrated to codeberg now, so https://codeberg.org/karrot/ is where development should happen. GitHub is just a mirror now.

nicksellen added a commit that referenced this pull request May 1, 2024
Emoji's broke because maxcdn stopped hosting the assets (see https://github.com/twitter/twemoji).

We can't just upgrade the library as Elon Musk's changes at twitter mean they don't support it any more. We could just change the asset URL but I thought we might as well update to a maintained fork (https://github.com/jdecked/twemoji), and get emoji v15 in the process 🫨

Backend: https://codeberg.org/karrot/karrot-backend/pulls/1295

Reviewed-on: https://codeberg.org/karrot/karrot-frontend/pulls/2714
Co-authored-by: Nick Sellen <git@nicksellen.co.uk>
Co-committed-by: Nick Sellen <git@nicksellen.co.uk>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants