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: Expandable rows not opening transaction #4048

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

mmioana
Copy link
Contributor

@mmioana mmioana commented Jan 7, 2025

Description

  • The purpose of this PR is to address the fact that the activity table rows do not open the action sidebar on mobile when clicked

Before

Screen.Recording.2025-01-07.at.19.26.23.mov

After

Screen.Recording.2025-01-07.at.19.25.15.mov

Testing

Please use a mobile simulator or just switch your browser to a width of < 1024px.

  • Step 1. Go to planex and scroll to the Recent activity table
    Screenshot 2025-01-07 at 19 28 11

  • Step 2. Click on any action and check that it opens the action sidebar

  • Step 3. Try expanding the action and check it still works as expected

  • Step 4. Check the column spacing is as expected

  • Step 5. Go to http://localhost:9091/planex/activity
    Screenshot 2025-01-07 at 19 28 19

  • Step 6. Repeat steps 2-4 and confirm everything still works as expected

Diffs

Changes 🏗

  • Use renderRowLink for the activity tables regardless of screen size

Resolves #3877

Fix: Expandable rows not opening transaction
@mmioana mmioana self-assigned this Jan 7, 2025
@mmioana mmioana marked this pull request as ready for review January 7, 2025 18:28
@mmioana mmioana requested a review from a team as a code owner January 7, 2025 18:28
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 on this, I am now able to open the action!

Screen.Recording.2025-01-08.at.11.52.15.mov

I just wanted to clarify something on the original issue:

When a row is selected on mobile when an individual table row is expanded, the action should open

I'm not sure if this means when a row is collapsed, that tapping on the row should expand it, then when it is expanded, tapping again should open the action.

With the current implementation, when a row is collapsed, tapping on the row opens the action, you have to tap specifically on the arrow to expand / collapse the row.

I think the current implementation works well, I just wanted to clarify what is expected from the original issue.

I also noticed tapping in the "details" section of the row does nothing, you have to specifically tap on the main part of the row itself to open the action. Again, not sure if this is an issue or not.

@mmioana mmioana marked this pull request as draft January 8, 2025 16:40
@mmioana
Copy link
Contributor Author

mmioana commented Jan 8, 2025

@iamsamgibbs thanks for your review! I'm putting this back in Draft until I clarify with @melyndav how it should actually behave

Fix: Update colony action row clickable area
@mmioana
Copy link
Contributor Author

mmioana commented Jan 13, 2025

Hey @iamsamgibbs I have clarified with @melyndav and on mobile, the area which should trigger the opening of the action sidebar should be as following

Action not expanded
Screenshot 2025-01-13 at 14 08 18

Action expanded
Screenshot 2025-01-13 at 14 08 25

Only clicking on the expand icon should toggle the extra content.

And on desktop the behaviour should remain as it is.

Would appreciate if you could check again 🙏

@mmioana mmioana marked this pull request as ready for review January 13, 2025 13:15
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.

Perfect! Thanks for clarifying this and getting it all fixed up, it's working very nicely now!

Screen.Recording.2025-01-13.at.14.13.09.mov

Desktop is unaffected:

Screen.Recording.2025-01-13.at.14.13.37.mov

Copy link
Contributor

@rumzledz rumzledz left a comment

Choose a reason for hiding this comment

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

All good on my end @mmioana ! 🚀

open-close-tx

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 both the activity feed on the dashboard, as well as the one in the activity feed page. Both work as expected. Nicely done! 🥇

iPhone-14-Plus-localhost-mnprxmlg2h5w42.webm
iPhone-14-Plus-localhost-kxdc-e-36uxyyr.webm

@mmioana mmioana merged commit 6b6f5a2 into master Jan 14, 2025
2 checks passed
@mmioana mmioana deleted the fix/3877-expanded-table-rows branch January 14, 2025 10:06
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.

[PROD] Native Mobile: Expanded tables rows not selectable
4 participants