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

Fix: Findings regarding table refactoring #4047

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

mmioana
Copy link
Contributor

@mmioana mmioana commented Jan 7, 2025

Description

Testing

TODO: Let's test these issues are fixed.

Note

Make sure to start a fresh environment using node scripts/create-data.js --timeout 100 --coloniesCount 12

Manage tokens

  • Step 1. Create a Manage tokens action
  • Step 2. Copy the token addresses from the other colonies and add them to the table until there are more than 10
  • Step 3. Check the padding bottom is not shown for mobile/tablet/desktop

Screenshot 2025-01-07 at 19 08 55
Screenshot 2025-01-07 at 19 09 12

Split payment

  • Step 1. Create a Split payment action
    Screenshot 2025-01-07 at 18 56 24

  • Step 2. Add more than 10 recipients

  • Step 3. Check pagination is not shown
    Screenshot 2025-01-07 at 18 57 35

  • Step 4. Switch to a mobile/tablet view

  • Step 5. Check Percent is shown for each row

  • Step 6. Check the footer amount and percent are right-aligned and have a right padding
    Screenshot 2025-01-07 at 18 57 54

  • Step 7. Complete the action and check steps 3-6
    Screenshot 2025-01-07 at 19 00 05
    Screenshot 2025-01-07 at 19 00 13

Edit colony

  • Step 1. Create a Edit colony details action
  • Step 2. Switch to a mobile/tablet view
  • Step 3. Scroll to the social links table and check the last item has a rounded left-bottom border
    Screenshot 2025-01-07 at 19 04 30

Resolves #4046

@mmioana mmioana self-assigned this Jan 7, 2025
@mmioana mmioana force-pushed the fix/4046-table-refactoring-findings branch from 2fdbd9e to 0a07e8e Compare January 7, 2025 18:10
@mmioana mmioana marked this pull request as ready for review January 7, 2025 18:11
@mmioana mmioana requested a review from a team as a code owner January 7, 2025 18:11
Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

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

Nice work fixing up these tables, this all looks good!

I've noticed one issue with the manage tokens table when on mobile view. Seems to only happen when you have 9 or more tokens selected.

Screenshot 2025-01-08 at 14 00 29

Everything else looks good though!

Screenshot 2025-01-08 at 13 49 06

Screenshot 2025-01-08 at 13 49 20

Screenshot 2025-01-08 at 13 56 07

Fix: Findings regarding table refactoring
@mmioana mmioana force-pushed the fix/4046-table-refactoring-findings branch from 0a07e8e to c736b32 Compare January 8, 2025 20:44
@mmioana
Copy link
Contributor Author

mmioana commented Jan 8, 2025

Hey @iamsamgibbs thank you for your review 🙌 I fixed the issue with the Manage tokens action on mobile. I did notice this issue appeared no matter the table size, but was caused by the token name length - as in USDC for Local Development

@mmioana mmioana requested a review from iamsamgibbs January 8, 2025 20:58
Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and detective work, this looks good now!

Screenshot 2025-01-13 at 11 22 06

Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

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

Tested all cases both in desktop and mobile and all seem to render just fine. Nicely done Ionana 💯

Manage Tokens:

Screenshot from 2025-01-13 21-10-04
Screenshot from 2025-01-13 21-10-22

Split Payment:

Screenshot from 2025-01-13 21-12-46
Screenshot from 2025-01-13 21-13-01
Screenshot from 2025-01-13 21-14-35
Screenshot from 2025-01-13 21-14-43
Screenshot from 2025-01-13 21-14-53
Screenshot from 2025-01-13 21-15-03
Screenshot from 2025-01-13 21-15-21
Screenshot from 2025-01-13 21-15-29
Screenshot from 2025-01-13 21-15-32

Edit Colony:

Screenshot from 2025-01-13 21-16-57
Screenshot from 2025-01-13 21-17-06

Copy link
Contributor

@bassgeta bassgeta left a comment

Choose a reason for hiding this comment

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

Nice one, table refactor is now 100% done 😎
Paddings not shown on manage tokens ✔️
image
image

Split payment pagination isn't shown ✔️
image
Percentages shown per row and in the footer ✔️
image
Completed action ✔️
image
image
Corners status on edit colony: rounded 🟢
image

🚢

@mmioana mmioana merged commit 1a07c44 into master Jan 15, 2025
2 checks passed
@mmioana mmioana deleted the fix/4046-table-refactoring-findings branch January 15, 2025 11:23
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.

[Tables]: Small findings after refactoring
4 participants