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

Display map samples tab in dataset page #2162

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

luistoptal
Copy link
Collaborator

@luistoptal luistoptal commented Jan 2, 2025

Pull request for issue: #718

  • Added new tab to the dataset page "Samples Map" which is displayed only if the dataset has "locations"
    http://gigadb.gigasciencejournal.com/dataset/100020
    image
  • Refactored the code related to the map to make it more reusable and maintainable
  • Updated map popup styles

How to test?

  • Navigate to http://gigadb.gigasciencejournal.com/dataset/100020 and check that the Samples Map tab exists and displays a map
  • Optionally, for a more complete view in protected/views/shared/_mapbrowse.php set the USE_TEST_LOCATIONS to true and refresh the map. I have left this test for reviewers to have an easy time seeing a more complete map, but I think it would be ok to remove it after this is reviewed
  • Note that you can now click outside the popup to close it

How have functionalities been implemented?

  • Abstracted the map widget to a reusable partial protected/views/shared/_mapbrowse.php and most of the code was refactored to protected/js/map-browse/index.js so that we can reuse the map in multiple views
  • The code in the SiteController that was in charge of building the locations array for the map is moved to a helper component protected/components/SampleLocationHelper.php so that it can be reused in the DatasetController
  • Cleaned up styles in less/modules/map.less mainly to make the popup look better
  • All the code refactored does the same as before, I just cleaned it up a bit and made html selector access more maintainable

Any issues with implementation?

  • I added a generateRandomLocation function that adds mock locations for testing. I would prefer to simply have more test data in local dev
  • The samples map is not accessible because the popups are not keyboard accessible. As far as I know there is no simple way around that provided by OpenLayers. I believe a UI which provides an alternative to the map would be a good idea. It depends on the goal of the map. If the goal is to let the user know samples by region then we could display a list of samples grouped by region and provide the links to the datasets
  • the popup links looks like this http://dx.doi.org/10.5524/${dataset} but link / redirect to the production gigadb website (from the local dev site). It looks like we redirect in the controller. It is not a big problem but for consistency I think it might be better to redirect to the current domain
  • I expect YII_DEBUG to be true in test environments and false in production but it would be good to confirm. I would like to use an environment variable that is only true on local dev

Any changes to automated tests?

  • I added a test to check for the presence of the map tab
  • As of now I cannot run acceptance tests, I run into
Db: SQLSTATE[08006] [7] invalid integer value "%GIGADB_PORT%" for connection option "port" while creating PDO connection  

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Tasks To Do
Development

Successfully merging this pull request may close these issues.

1 participant