feat: improve consistency between Tree Detail and Planter Detail dialogs #10 #143
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
chromium-52
requested review from
gwynndp,
luacmartins and
nmcharlton
as code owners
nmcharlton
requested changes
Aug 9, 2021
There was a problem hiding this comment.
Fantastic work! Just a couple of comments on the changes.
Some other suggestions for adjacent changes:
- I think we can get rid of the slide up transition – it looks a little odd when placed next to the sliding drawer.
- Can we also not clear the capture on
handleCloseinVerify.js? That will give a smooth when closing the dialog.
| </Box> | ||
| </Grid> | ||
| <Grid item> | ||
| <IconButton onClick={() => props.onClose()}> |
There was a problem hiding this comment.
Clicking the Close button throws an exception:
Uncaught TypeError: props.onClose is not a function
| > | ||
| <OptimizedImage | ||
| src={renderCapture.imageUrl} | ||
| width={320} |
There was a problem hiding this comment.
I think we can make the capture image bigger now there's a bit more room for it.
We have three options, any of which would work well:
- Dynamically calculate what size of image would fit in the space and request that using the
OptimizedImagecomponent - Drop
OptimizedImagefor the full-size image and just shrink the original image to fill the space - Set the
OptimizedImagewidth to600
…apture image larger
|
Thank you so much for the comments! I've made the above changes; please let me know if I misunderstood anything or if I can improve further! |
nmcharlton
approved these changes
Aug 14, 2021
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.
Resolves #10