-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[$500] Dev: Web - Inconsistence space between input fields in #Rooms #27604
Comments
ProposalPlease re-state the problem that we are trying to solve in this issue.Margin below room name is more than the bottom margin for other fields below What is the root cause of that problem?We are using a different margin here
which is different here
What changes do you think we should make in order to solve the problem?We should use the same margin What alternative solutions did you explore? (Optional) |
Triggered auto assignment to @puneetlath ( |
Job added to Upwork: https://www.upwork.com/jobs/~01df923e800f40dae5 |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to @laurenreidexpensify ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee ( |
ProposalPlease re-state the problem that we are trying to solve in this issueInputs have different distances between them What is the root cause of that problem?Different styles in inputs What changes do you think we should make in order to solve the problem?In this component we have the some inputs which have some different styles App/src/pages/workspace/WorkspaceNewRoomPage.js Lines 171 to 206 in 76c1559
To reduce potential errors, it would be a good idea to create a constant for styles so that you can change styles in only one place Or create a wrapper component for the inputs What alternative solutions did you explore? (Optional)NA |
@jjcoffee what you reckon about the proposal above? |
Here is a proposal as well just in case it is missed @jjcoffee |
@c3024's proposal looks good - correct RCA and solution. I'm not 100% sure if this wasn't intentional, so we might want some design eyes to verify that we want to get rid of the spacing here. @shawnborton or @dannymcclain Are we good to reduce the margin below the "Room name" input to match the other inputs? 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @tylerkaraszewski, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
Looking at some other mocks with multiple inputs, I actually think the spacing between the @shawnborton can verify but I'm pretty sure we want to add space here, not remove it. |
@dannymcclain Ah so you're saying we should increase the spacing on all the other inputs instead? |
I believe so! |
Edit: This is a different address page and different pages have different spacing. So we might use |
Yup, that makes sense to me. Though I think @dangrous is working on revamping this page right now so maybe we want to just close this issue out in favor of the work DGR is doing? |
Yep I can tackle that in my PR. Do you mind commenting with what it should be (looks like mb5 I think?) on this PR? |
Can do. Okay, @laurenreidexpensify let's close this one down. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@laurenreidexpensify i think this is eligible for reporting bonus because it will be handled in other PR after i reported this issue |
Bump @laurenreidexpensify |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Expected Result:
Space should be consistence between input fields
Actual Result:
Space under Room name is more compare to other
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: Dev 1.3.70.5
Reproducible in staging?: n
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screen.Recording.2023-09-16.at.10.37.38.AM.mov
Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694840876055859
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: