Location Modal #505
Location Modal #505
Conversation
|
[diff-counting] Significant lines: 152. |
| flex-direction: column; | ||
| font-size: 0.875rem; | ||
| height: 31px; | ||
| width: 224px; |
There was a problem hiding this comment.
Hi Raissa, I thought you did a great job on the PR and styling of the modals. One suggestion I would make is that in the css for the modal, you should use rem instead of px for width and height in general. This suggestion was originally from Desmond for my PR, and he explained that rem tends to be better for responsiveness and different screen sizes.
Here a converter online:
https://nekocalc.com/px-to-rem-converter
| <div> | ||
| <Button className={styles.submit} type="submit"> | ||
| {submitButtonText} | ||
| </Button> | ||
|
|
||
| <Button className={styles.clearButton} type="submit"> |
There was a problem hiding this comment.
Hi Raissa, great job with this pull request! Overall it looks good to me, but one thing to note is that I think that the "Clear All" button should be type="reset" instead of type="submit" so that it resets the inputs.
- Formatted input boxes and titles according to Figma designs - Add appropriate error messages with accurate functionality - Add Clear All and Back buttons
Summary
This pull request is for implementing the redesign of the Location Modal. So far, the following changes have been made:
This PR is incomplete, please read the Notes!
Test Plan
Screen.Recording.2024-04-15.at.9.50.12.PM.mov
Here, I demonstrate how the Clear All button functions. I also show the error messages appearing when the name or address is not inputted. According to the Figma design, there is no error message if the pickup/dropoff information is blank, so I removed its requirement.
Notes
Important: The clear all button doesn't work for editing a location that's already been created, as shown in the screen recording below. Also, when an already created location is edited, the website needs to be reloaded in oder to show the changes (we want the changes to show up immediately). Back button is also not yet implemented. These issues are not addressed in this PR.
Screen.Recording.2024-04-15.at.9.53.26.PM.mov
Breaking Changes
None