This repository has been archived by the owner on Feb 1, 2024. It is now read-only.
Add full-width embedded map support #1352
Merged
Conversation
This file contains 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
caseycesari
approved these changes
May 26, 2021
src/app/src/util/util.js
Outdated
| left:0;width:100%;height:100%;" | ||
| title="embedded-map"></iframe> | ||
| </div> | ||
| </div>` |
There was a problem hiding this comment.
Consider changing the indentation over so that this block lines up with the block below.
There was a problem hiding this comment.
Good call, thanks
| const DEFAULT_WIDTH = '1000'; | ||
| const DEFAULT_HEIGHT = '800'; | ||
| export const DEFAULT_WIDTH = '1000'; | ||
| export const DEFAULT_HEIGHT = '800'; |
There was a problem hiding this comment.
This may be for a follow-up card, but do we want to set a minimum as well? The map is unusable at small sizes.
There was a problem hiding this comment.
@caseycesari My thought here is that, since users will have direct visual feedback that it looks terrible if they set it at a small size, we can let them self-police their sizing.
Adjusts the embedded map preview and sample code to allow users to set the embedded map to full width, and implements sensible defaults when moving between full width and set width. Per a conversation with Scott, some small changes have been made to the styling of the size section of the embedded map settings. As part of updating the iframe code, also fixes a type in the iframe src url.
TaiWilkin
force-pushed
the
tw/full-width-iframe
branch
from
97ec214
to
90a39f3
Compare
|
Thanks for the review! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Overview
Adjusts the embedded map preview and sample code to allow users
to set the embedded map to full width, and implements sensible
defaults when moving between full width and set width.
Per a conversation with Scott, some small changes have been made to the
styling of the size section of the embedded map settings.
The width and height text inputs were not allowing users to completely clear them, which made it difficult to enter an entirely new size. There is now a validation message presented in response to empty text inputs instead.
Connects #1311
Testing Instructions
./scripts/serverChecklist
fixup!commits have been squashed