-
Notifications
You must be signed in to change notification settings - Fork 984
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
Activity tab on collectible page #18220
Conversation
Jenkins BuildsClick to see older builds (13)
|
Hi @vkjr. I've been working on the same page on the About tab. I followed a different approach for structuring the codes. You can see my branch which is not finished yet btw here cc @J-Son89 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very focused PR! it looks good, just left smal comments 👍
{:id :about | ||
:label (i18n/label :t/about) | ||
:accessibility-label :about-tab}]}]) | ||
(def tabs-data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool you managed to change a defn
to a def
👍
[info chain-id] | ||
[traits-section traits]] | ||
:activity [activity-section] | ||
[rn/view])]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it just the same as just returning nil
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have an empty view here? Either we should return something or not 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think nil should be good)
:style style/tabs | ||
:scrollable? true | ||
:default-active @selected-tab | ||
:data tabs-data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead of calling them tabs-data
we can give a name more related to what kind of tabs they are, e.g. activity-tabs
or something like that. wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that would be more convenient for global search!
:type :network | ||
:network-logo (quo.resources/get-network :ethereum) | ||
:network-name "Mainnet"} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trailing }
@J-Son89, it is my mistake. In the heading should be the name of the collection, not the description. Probably I messed that after |
fb8cd50
to
8f902f2
Compare
8f902f2
to
b0d08d5
Compare
56% of end-end tests have passed
Failed tests (17)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Passed tests (27)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
|
88% of end-end tests have passed
Failed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Expected to fail tests (5)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (42)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
@vkjr thanx for the PR. e2e are okay! Ready for merge if no manual qa required. |
92ff4df
to
62c2179
Compare
62c2179
to
46a44f5
Compare
fixes #17317
Summary
This PR implements activity tab on collectible page. So far with mock data.
Figma designs
status: ready