-
Notifications
You must be signed in to change notification settings - Fork 21
[DO NOT MERGE] Add map block #4775
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
Draft
andysellick
wants to merge
17
commits into
main
Choose a base branch
from
add-map-block
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a1d99a7
to
9649255
Compare
9649255
to
9cc5fe4
Compare
9cc5fe4
to
126dcd8
Compare
126dcd8
to
02d257a
Compare
Closed
f5357a0
to
816b50c
Compare
816b50c
to
cc61261
Compare
cc61261
to
0695658
Compare
5c4971c
to
29306bc
Compare
29306bc
to
5b7ba5c
Compare
d2fa466
to
dc840f5
Compare
- had to get the title of the popup on popup open - can't store a value in the ga4 object, so having to read the popup contents and look for the title using an attribute
d97ce45
to
dd93bb7
Compare
dd93bb7
to
af8798a
Compare
af8798a
to
e7c921e
Compare
e7c921e
to
5debff2
Compare
5debff2
to
da46853
Compare
da46853
to
79a6be2
Compare
79a6be2
to
d6d6643
Compare
d6d6643
to
16784ad
Compare
16784ad
to
b2aede4
Compare
b2aede4
to
49773d8
Compare
9f14596
to
8f68745
Compare
- remove old map code and add new module and test file - add delayed check for cookie consent with analytics - add styling to make map error state stand out a little more
- reduce spacing on mobile for popups, prevent collision with headings and close icons - set maximum width of popups to prevent overlap on mobile - set maximum height of map on mobile to not be more than screen height, to help with scrolling - remove in-house leaflet CSS and use one from package instead, move overrides into main stylesheet
8f68745
to
535828d
Compare
- this bit of the code changed the icons for popups to a slightly different appearance when the popup is opened - however the lines of code that control this behaviour are quite buried in the code and extremely hard to provide test coverage for, which caused the test coverage to be below the required level - additionally the visual change with these icons is very subtle and provides little benefit, particularly since the popup appearing is a highly visual indication that something has changed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Adds a custom map block to the
landing_page
structure.Partly based on initial work in #4748
Why
Visual changes
Trello card: https://trello.com/c/zVqSZM6D