M-6 Activity Create UI #12
Conversation
There was a problem hiding this comment.
I have made the title field required so that people can quickly add a new activity and add more information later.
I am unsure about how we are handling images. Are we storing images in the cloud and storing their CDN in the database? Currently, once images are added and the submit button is clicked, it stores image information like the example below:
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Oh sorry about that! I just fixed the error! |
| import { PrimaryButton } from '@/components/button' | ||
| import { createActivitySchema, createActivityResolver } from './schema' | ||
|
|
||
| export const Form = () => { |
There was a problem hiding this comment.
I think it's better to say FormActivity which is more clearly and easier to search!
There was a problem hiding this comment.
I changed the name to FormActivity!
app/activity/create/page.tsx
Outdated
| <Container | ||
| maxW={{ base: '100%', md: 'container.md' }} | ||
| pt={{ base: '20px', md: '30px' }} | ||
| pb={{ base: '40px', md: '80px' }} | ||
| margin="auto" | ||
| > | ||
| <Container> | ||
| <Heading as={'h1'} fontSize={{ base: '2xl', md: '4xl' }}> | ||
| Create Activity | ||
| </Heading> | ||
| </Container> | ||
| <Form /> | ||
| </Container> |
There was a problem hiding this comment.
<Container /> is usually one per page. Please change to one <Container /> with container.md for PC as you already use for top layer. You don't need to use <Container /> in Form component too
There was a problem hiding this comment.
I removed the extra container tags in page.tsx under the create directory, and removed container tags in form.tsx under the components directory. Could you please take a quick look and let me know if it looks good?
| mt={{ base: '30px', md: '40px' }} | ||
| isInvalid={!!errors.timeFrom} | ||
| > | ||
| <FormLabel>Time From</FormLabel> |
There was a problem hiding this comment.
Activity can be not only one day so I think we need to show date too...
If ChakraUI doesn't provide such the components, we need to make ourselves. Using like React Datepicker...
There was a problem hiding this comment.
I used Date and time Picker Chakra provides so users can add date and time in one input field!
https://chakra-ui.com/docs/components/input/usage#:~:text=Input%20Methods%20other%20than%20Text
There was a problem hiding this comment.
Oh that's great that Chakra has already have it! But the theme colour is difference so can you change it from blue to primary?
There was a problem hiding this comment.
As I mentioned in the Slack channel, the 'dateTime-local' input type is not supported in some browsers (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/datetime-local).
As an alternative, I used the React datetime picker (https://www.npmjs.com/package/react-datetime-picker).
I included styles in the TSX file to overwrite the default CSS and tailor the calendar to our color theme.
Screen.Recording.2023-12-29.at.15.57.03.mov
| <Box | ||
| {...getRootProps()} | ||
| textAlign="center" | ||
| mt={{ base: '30px', md: '40px' }} |
There was a problem hiding this comment.
Bedore uploading images, I think this space is too much 👀
There was a problem hiding this comment.
I added margin only when there is at least one image, so it doesn't appear like there is too much space without images.
| mt={{ base: '30px', md: '40px' }} | |
| <Box | |
| {...getRootProps()} | |
| textAlign="center" | |
| mt={selectedImages.length !== 0 && { base: '30px', md: '40px' }} | |
| > | |
| <PrimaryButton variant="outline"> | |
| <input {...getInputProps()} /> | |
| Add Image | |
| </PrimaryButton> | |
| </Box> |
There was a problem hiding this comment.
If you think about the consistency with the other form fields, it's better to have same margin top between label and form. So the other fields have 8px for margin top, the image also should have 8px. And I think it's better to align-left for consistency! The final pic is what I think ideal style.
There was a problem hiding this comment.
I adjusted the spacing! I still added 40px margin bottom when images are inserted so there is good amount of space between the title and images.
 |
 |
|---|---|
 |
 |
| <Input | ||
| type="text" | ||
| placeholder="Asakusa Temple" | ||
| {...register('title')} | ||
| /> |
There was a problem hiding this comment.
For dark mode, I think it's hard to see the input components, so could you please change to like design? You can change in theme file or make input component if you think it's better to maintain!
There was a problem hiding this comment.
Thank you so much for pointing that out! I created InputForm and TextareaForm under the input components and added color for dark mode. Is it a good way?
There was a problem hiding this comment.
Thank you, it is good! But could you modify the theme colour from blue to primary? You can do like search input, or you can change it in theme file
There was a problem hiding this comment.
I added primary.600 for focus border color
| @@ -0,0 +1,17 @@ | |||
| import { zodResolver } from '@hookform/resolvers/zod' | |||
There was a problem hiding this comment.
schema is not component directory! You can make schema directory
app/activity/create/page.tsx
Outdated
| import {Form} from "./components" | ||
|
|
||
|
|
||
| export default function Create() { |
There was a problem hiding this comment.
It's better to name like CreateActivityPage which is more clear!
profile and error handling using Suspense.
Description
giphy.1.mp4
Fixes # (issue)
Type of change
Screenshot