-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-10-14] [$250] Increase table row and decrease table row gap #49582
Comments
Triggered auto assignment to @alexpensify ( |
Edited by proposal-police: This proposal was edited at 2024-09-23 07:25:22 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Increase table row and decrease table row gap What is the root cause of that problem?new feature What changes do you think we should make in order to solve the problem?
ResultWhat alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2024-09-23 08:09:52 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Increase table row and decrease table row gap What is the root cause of that problem?Right here the Categories Table list item had total height of 52px with padding top and bottom 16px including content which is not correct, we can increase the paddingVertical to 22px so the height will be 64px App/src/components/SelectionList/TableListItem.tsx Lines 45 to 49 in bbc0919
Lines 4639 to 4647 in bbc0919
And for the gap here we use mb3 which is 12px containerStyle={styles.mb3} instead of mb2 (8px)App/src/components/SelectionList/TableListItem.tsx Lines 45 to 49 in bbc0919
App/src/components/SelectionList/Search/TransactionListItem.tsx Lines 35 to 39 in bbc0919
App/src/components/SelectionList/Search/ReportListItem.tsx Lines 123 to 127 in bbc0919
App/src/components/SelectionList/ChatListItem.tsx Lines 58 to 64 in bbc0919
What changes do you think we should make in order to solve the problem?We can increase the padding vertical (top and bottom) to 22px so the total height will be 64px instead of 52px Lines 4639 to 4647 in bbc0919
And for mb3 containerStyle={styles.mb3} we can change to mb2 (8px) for gap space each item
What alternative solutions did you explore? (Optional) |
Proposal Updated
|
@dubielzyk-expensify - are we ready to assign this as |
Job added to Upwork: https://www.upwork.com/jobs/~021838357676436244247 |
Nobody assigned. Added the label |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ikevin127 ( |
Proposal Updated
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Increase table row and decrease table row gap What is the root cause of that problem?new feature What changes do you think we should make in order to solve the problem?For reducing the gap between rows need to change style containerStyle={styles.mb3} to App/src/components/SelectionList/TableListItem.tsx Lines 45 to 49 in bbc0919
For increasing table row height need to change style Lines 4639 to 4647 in bbc0919
paddingVertical should be 16px to 22px so the style should be
What alternative solutions did you explore? (Optional) |
@ikevin127 when you get a chance, can you please review these proposals? Thanks! |
This comment was marked as outdated.
This comment was marked as outdated.
@abzokhattab's proposal looks good to me. The proposed solution fulfils the expectations of the issue requirements which mention setting a minimum height of 64px and decreasing the margin between items. I was testing whether increasing the padding or setting
🎀👀🎀 C+ reviewed |
Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @ikevin127 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @abzokhattab 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Weekly Update: The PR is still in review |
Update: waiting for this PR to go to production |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.45-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-10-14. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
Regression Test Proposal
Do we agree 👍 or 👎. |
Payouts due: 2024-10-14
Upwork job is here. I've completed the payment process in Upwork. |
We recently decided to do some minor tweaks to our tables and would like that reflected in the product. The changes are as follows:
64px
8px
instead of12px
.Screenshots (before left, after right)
This should apply to all tables (search, members, categories, tags, etc). cc @Expensify/design
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @alexpensifyThe text was updated successfully, but these errors were encountered: