Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restyled custom events modal #1044

Merged
merged 1 commit into from
Nov 22, 2024
Merged

Conversation

xgraceyan
Copy link
Contributor

Summary

Fixed spacing and layout of event modal (Changed modal -> Previous modal)

image
image

Fixed responsive issue with day selector when using mobile view (After - Before)
image
image

Test Plan

Make sure modal works properly and there are no visual issues on any viewport

Issues

Building search variant is not consistent with the other inputs (filled vs. outlined); however, the filled version of building select looks better in its other appearance in the map view
Perhaps we could pass the variant as props to the component so the building select can be outlined in the custom events modal while also filled in the map view?

Closes #1025

Fixed spacing and layout of event modal
@xgraceyan xgraceyan changed the title Restyled modal Restyled custom events modal Nov 20, 2024
@MinhxNguyen7
Copy link
Member

@xgraceyan this looks ready for review. Is it? If not, no problem. Was just wondering.

@xgraceyan xgraceyan marked this pull request as ready for review November 21, 2024 22:21
@xgraceyan
Copy link
Contributor Author

Yes, it's ready for review! Sorry, I forgot to change the status of the PR from a draft.

Copy link
Member

@MinhxNguyen7 MinhxNguyen7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@MinhxNguyen7 MinhxNguyen7 merged commit 41552e7 into main Nov 22, 2024
6 checks passed
@MinhxNguyen7
Copy link
Member

I'll just comment that the "Issues" section in the PR description is for related GitHub issues. Issues with the PR itself should go in the description.
Also, I'll let it slide for now, but the "test plan" should be a little more detailed. E.g., check on 16x9 desktop, tablet, and small phone (Galaxy S8+).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restyle Custom Events Modal
2 participants