Leon and Oleksandr add interactive map #1503
Conversation
Add new protected route which the interactive map of team locations will eventually be accessed from. Only users who can see the Reports dropdown can access this route. Also add the new files to .prettierignore to comply with Frontend PR#938
This data will eventually be displayed when clicking markers on the map.
The Team Locations page is intended to be a non-scrollable page, so the back to top button isn't needed here and messes with the layout. It's added back when the component is unmounted so it still works on other pages.
Add map options for better UX, such as starting the map at a reasonable zoom level, setting the initial center to be a reasonable place, setting panning boundaries, etc.
Display markers corresponding to each user. On click, a popup appears showing their name, job title, and location. Currently the locations of each marker are randomly chosen, because geocoding functionality hasn't been implemented yet.
- Add logic to pass each user's human-readable location to a geocoding API and retrieve the corresponding latitude and longitude of the location. To reduce number of API calls, an API call is not made if the location is obviously invalid (ex. empty string.) For testing purposes only a maximum of 100 API calls are made currently, since there is a limit of 2500 calls per day with the test API key. - Also, add logic to display markers at the user's actual location in the world, since the correct latlng information has been obtained. Only users with a valid location are displayed on the map. - Change the marker appearance to that of a circle.
Add marker clustering functionality. This is necessary since it is possible for multiple people to have the same location, thus their markers will overlap. The marker clustering plugin prevents this. As a bonus, it also shows how many markers are represented by a cluster. - New package react-leaflet-cluster
Add caching functionality via localStorage to reduce the number of network requests and dramatically speed up rendering of markers.
change a localStorage.getItem() call to use correct arguments
Since we are limited to only 2500 requests/day for the geocoding API in production, requests must be kept at a minimum. Here's some of the facts: - Everybody who has ever contributed to OneCommunity will be displayed on the map. This means a lot of different locations. A request for each unique location will contribute a lot towards this limit. - A public facing version of this map is planned. and for every new user who views the map, there will be a ton of API calls made since their browser has not cached the data yet. - The cache is implemented via localStorage, and if it is cleared, a bunch of API requests will be made again. This means somebody could - intentionally or unintentionally, make us reach our limit in the span of a few minutes. Therefore, the current approach of caching the data in the browser as a rate limiting technique is not feasible given our siutation. Instead, a better way is to store the latlng as a property of a user's profile in the database and only call the API to update the latlng when a new user is created or when their location changes. Then, when someone visits the map, there are zero API calls made since the locations have been precomputed and stored in the backend. Therefore: - remove caching logic - add mock data similar to how the new response from the backend would look like
Refactor logic so that the explicit conditional check to render or not render a marker based on whether the location is valid or not is removed, and instead the array of users is filtered beforehand to only include users with valid locations. Makes the JSX cleaner. Also, added an example user with no valid location to show how the map code handles it.
Change it to to something more accurate.
Add new protected route which the interactive map of team locations will eventually be accessed from. Only users who can see the Reports dropdown can access this route. Also add the new files to .prettierignore to comply with Frontend PR#938
This data will eventually be displayed when clicking markers on the map.
The Team Locations page is intended to be a non-scrollable page, so the back to top button isn't needed here and messes with the layout. It's added back when the component is unmounted so it still works on other pages.
Add map options for better UX, such as starting the map at a reasonable zoom level, setting the initial center to be a reasonable place, setting panning boundaries, etc.
Display markers corresponding to each user. On click, a popup appears showing their name, job title, and location. Currently the locations of each marker are randomly chosen, because geocoding functionality hasn't been implemented yet.
- Add logic to pass each user's human-readable location to a geocoding API and retrieve the corresponding latitude and longitude of the location. To reduce number of API calls, an API call is not made if the location is obviously invalid (ex. empty string.) For testing purposes only a maximum of 100 API calls are made currently, since there is a limit of 2500 calls per day with the test API key. - Also, add logic to display markers at the user's actual location in the world, since the correct latlng information has been obtained. Only users with a valid location are displayed on the map. - Change the marker appearance to that of a circle.
Add marker clustering functionality. This is necessary since it is possible for multiple people to have the same location, thus their markers will overlap. The marker clustering plugin prevents this. As a bonus, it also shows how many markers are represented by a cluster. - New package react-leaflet-cluster
|
Thank you @ptrpengdev
|
Got it. Will test. Thank you! |
There was a problem hiding this comment.
hi @xaanders
the map renders perfect on my local environment and the number of countries show correctly this time. when in admin and volunteer login, the edit of team number are prohibited.
when I try to create team on the local machine, it seems the new team is never saved. could you help me check how to create a team to test functions for onwers?
Thank you!
|
Hi @xaanders, I have tested the functionalities with the Owner role except for adding users, and everything else worked perfectly. Great work! Additionally, I wanted to share my personal thought: it would be great to have tooltips or caption text over the location field of the "Adding New User" form in the map's "Add Person" feature as an enhancement. I encountered difficulty using the "get location" function. I tried entering locations manually like Boston, MA, California, USA, and copying locations from existing users, but all attempts failed. It would be helpful to have suggestions of input data format appear over the input text field for the new user. Thank you! I have attached the video of the Owner role testing. This video includes user searching in the search bar, editing user data, interacting with the map, using user list functions, and displaying the count of countries. video1831642007.mp4 |
|
Hey, I reviewed the code and tried testing by following the steps. I was able to modify users from the map as well as form the user list. I tried to add the user but I always got the Timezone key not found error even after adding the timezone variables as mentioned in the description. I used the Geocoding API key which is available for free. Please let me know if there is anything else that I need to do to make it run. Thank you 1503_599.mp4 |
There is no need to create a team to test functionality |
|
@ptrpengdev @palakgosalia you can check it in the dev tools with redux dev tools if it exists in your global state |
There was a problem hiding this comment.
Hi @xaanders I have tested this PR with PR 599 locally .
Review:
I successfully managed to add, edit, and delete users without encountering any errors. However, I noticed that sometimes, after navigating to other buttons and returning, the search and add user buttons are missing. Despite attempting to refresh the page, the issue persists(Video 2)
Screen.Recording.2023-11-18.at.4.26.27.PM.mov
Screen.Recording.2023-11-18.at.4.28.29.PM.mov
There was a problem hiding this comment.
Hey @xaanders ,
Looking good overall! 🙌 I’ve tested the following, and everything went smoothly:
- A user can be added through the “Add person” button.
- Existing users can be edited/deleted through the “Users list” or the searched results.
- Existing users can be found/edited/deleted through the map.
- The search bar works properly while inputting either names or locations.
- Updating an existing HGN user’s profile will update the map properly.
- Manager role users have no access to add/edit/remove/search users.
Not a big deal, but a potential minor improvement: The horizontal scroll bar within the search bar dropdown is not automatically showing up, making it a little bit hard to access the “Edit” buttons on the right-hand side.
PR1503+599.mp4
There was a problem hiding this comment.
Hi @xaanders, I thoroughly tested this pr. All the functionality works well as intended the users' coordinates are reflected well on the map and it is editable as well.
However, I realized a small flaw occurs when you're logged in as an Owner and then edit someone else's account in the slightest way. Then returning to the map the search bar and tools are gone although you are signed in as an owner. Looks like the redux state is storing the userprofile's role that you edited instead of the logged in user.
Screen.Recording.2023-11-22.at.5.16.22.PM.mp4
| const [editIsOpen, setEditIsOpen] = useState(false); | ||
| const [editingUser, setEditingUser] = useState(null); | ||
| const [searchText, setSearchText] = useState(''); | ||
| const role = useSelector(state => state.userProfile.role); |
There was a problem hiding this comment.
Change this to:
const role = useSelector(state => state.auth.user.role);
There was a problem hiding this comment.
It is a good one!
Thank you!
…unityGlobal/HighestGoodNetworkApp into leon_add_interactive_map
xaanders
dismissed stale reviews from robertoooc and KavyaAlla
Should be fixed
|
All functionality works. However, I think it would be a cool feature to search a user and automatically zoom to their saved location on the map when clicked on screencast-bpconcjcammlapcogcnnelfmaeghhagj-2023.11.24-14_04_03.webm |
There was a problem hiding this comment.
Hello, @xaanders and @leonzh2k! Great implementation of this cool feature! The functionalities work as expected:
1) Testing adding, editing, and removing the user from the map (marker/list) and from the existing user profile:
add_interactive_map_testing.mp4
2) Creating a new user and testing functionality by punching 10+ hours and without punching hours:
creating_new_user.mp4
3) Testing the functionality for Admin and Volunteer roles:
cannot.add.or.edit.or.remove.or.search.users.in.other.roles.mp4
4) Search Functionality:
The search functionality works excellent in most aspects. However, I noticed that only the first name is searchable. In the following test, observe how I do not receive search results when I search for 'Oleksandr Volunteer' although there is an user on the map with that name. Perhaps in the next update, the functionality can have a search query that retrieves the full name in its results.
search_functionality.mp4
Thank you!
Description
Related PRS
This frontend PR is related to the #599 backend PR.
Main changes explained:
How to test:
npm installandnpm run start:localto run this PR locallya) create new HGN user: and punch at least 10 tangible hours to the user
b) use existing one: go to user profile and update its location by getting timezone and saving
8. find user on the map and try edit through marker/list/search
9. go to user profile and compare
10. edit some info in the profile and compare to the map
11. login to any other user and confirm there is no way to add/edit/remove/search users
Screenshots or videos of changes:
map.mp4