feat: radix image upload #468
Conversation
kriskbx
force-pushed
the
feat/radix-image-upload
branch
from
81abedc to
1ea36c8
Compare
There was a problem hiding this comment.
Very straightforward and approachable code! ✨
The only issue I found is that the "Review" step shows a cropped version of my uploaded test image:
I expected the complete image to be shown here.
If you agree, feel free to implement the other style-related suggestions – they're not blocking though IMO.
And btw: The new UI looks fabulous!
| } | ||
| }, [isDialogOpen]); | ||
|
|
||
| useEffect(() => { |
| import styled from "styled-components"; | ||
| import { t } from "ttag"; | ||
| import { ImageUploadContext } from "~/components/CombinedFeaturePanel/components/FeatureImageUpload"; | ||
|
|
There was a problem hiding this comment.
Idea: With styled(), we could make the components below more readable:
| const ListItem = styled.li` | |
| img { max-width: 100%; } | |
| `; | |
| const Icon = styled(CheckIcon)` | |
| fill: var(--green-a10); | |
| stroke: var(--green-a10); | |
| width: 1.5rem; | |
| height: 1.5rem; | |
| flex-shrink: 0; | |
| `; | |
| const Pictogram = styled.figure` | |
| margin: 0; | |
| & > figcaption { | |
| text-align: center; | |
| } | |
| `; | |
| const Heading = styled.header` | |
| margin: 0; | |
| font-size: 1rem; | |
| `; | |
| <Flex asChild direction="column" gap="2"> | ||
| <ImageCriteriaList> | ||
| <Card asChild> | ||
| <li className="image-criteria-list__list-item"> |
There was a problem hiding this comment.
| <li className="image-criteria-list__list-item"> | |
| <ListItem> |
| <li className="image-criteria-list__list-item"> | ||
| <Flex gap="2" align="center"> | ||
| <CheckIcon className="image-criteria-list__icon" aria-hidden /> | ||
| <h3 className="image-criteria-list__heading"> |
There was a problem hiding this comment.
| <h3 className="image-criteria-list__heading"> | |
| <Heading> |
| {t`For example by showing entrances, toilets or a map of the site.`} | ||
| </Text> | ||
| <Grid columns="3" gap="2" mt="3" aria-hidden> | ||
| <figure class="image-criteria-list__pictogram"> |
There was a problem hiding this comment.
| <figure class="image-criteria-list__pictogram"> | |
| <Pictogram> |
…etc. – what do you think?
|
|
||
| .image-criteria-list__list-item { | ||
| img { | ||
| max-width: 100%; | ||
| } | ||
| } | ||
|
|
||
| .image-criteria-list__icon { | ||
| fill: var(--green-a10); | ||
| stroke: var(--green-a10); | ||
| width: 1.5rem; | ||
| height: 1.5rem; | ||
| flex-shrink: 0; | ||
| } | ||
|
|
||
| .image-criteria-list__heading { | ||
| margin: 0; | ||
| font-size: 1rem; | ||
| } | ||
|
|
||
| .image-criteria-list__pictogram { | ||
| margin: 0; | ||
|
|
||
| & > figcaption { | ||
| text-align: center; | ||
| } | ||
| } |
There was a problem hiding this comment.
| .image-criteria-list__list-item { | |
| img { | |
| max-width: 100%; | |
| } | |
| } | |
| .image-criteria-list__icon { | |
| fill: var(--green-a10); | |
| stroke: var(--green-a10); | |
| width: 1.5rem; | |
| height: 1.5rem; | |
| flex-shrink: 0; | |
| } | |
| .image-criteria-list__heading { | |
| margin: 0; | |
| font-size: 1rem; | |
| } | |
| .image-criteria-list__pictogram { | |
| margin: 0; | |
| & > figcaption { | |
| text-align: center; | |
| } | |
| } |
| </h3> | ||
| </Flex> | ||
| <Text color="gray"> | ||
| {t`By uploading this image, I hereby publish it in the public domain as renounce copyright protection: `} |
There was a problem hiding this comment.
| {t`By uploading this image, I hereby publish it in the public domain as renounce copyright protection: `} | |
| {t`By uploading this image, I hereby publish it in the public domain and renounce copyright protection: `} |
| } | ||
| } | ||
|
|
||
| .image-upload-dropzone__text--on-mobile { |
There was a problem hiding this comment.
A small comment where is this class name is determined would be ace here (Is this a feature of react-dropzone?)
| object-fit: cover; | ||
| } | ||
|
|
||
| .image-upload-review__overlay { |
There was a problem hiding this comment.
Same as above: My hunch is that this could be a separate const OverlayBox = styled(Box)… to stay in styled-componentsʼ idiomatics.
| .image-upload-review__preview-image { | ||
| width: 100%; | ||
| height: 100%; | ||
| object-fit: cover; |
There was a problem hiding this comment.
| object-fit: cover; | |
| object-fit: contain; |
This PR is a rewrite of the image upload UI using the radix-ui library.
before testing
Make sure to run
npm installand install newly added dependencies.known issues
There's a bug that causes some flickering when opening the actual dialog. I still suspect that something higher up in the component tree gets rerendered and causes this. In this particular case, this behaviour is even more frustrating as the entire state of the upload dialog gets reset when this happens, effectively removing the selecting image and jumping back to step one. It seems to be related to clicking/interacting with the Browser. Sometimes it also just works... 😄 Keep this in mind, when testing the PR. I will hunt down the rerender bug next and create a separate PR for this.