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.

Sign up for GitHub

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

Jump to bottom

[explorer] adds Preview Cards #1155

Merged
merged 31 commits into from
Apr 21, 2022
Merged

[explorer] adds Preview Cards #1155

merged 31 commits into from
Apr 21, 2022

Conversation

apburnie
Copy link
Contributor

Both Addresses and Objects and own a list of objects. These 'Owned Objects' are currently displayed as a list of object IDs. This PR replaces this string list with a series of clickable cards that gives a preview of what the object represents.

@apburnie apburnie force-pushed the explorer-previewcards branch from bc37060 to 33421ea Compare March 31, 2022 13:13
@apburnie apburnie marked this pull request as ready for review April 4, 2022 10:10
@apburnie apburnie requested a review from Clay-Mysten as a code owner April 4, 2022 10:23
@apburnie
Copy link
Contributor Author

apburnie commented Apr 4, 2022

This PR is now ready for review.

Example Views to Help the Review Process

Particularly useful UIs to take a look at (after running npm start):

Next Steps

The following are additional pieces of functionality which could be approached in separate PRs or included within this MR depending on reviewer feedback:

  • Adding a search function to enable the user to search across owned objects.
  • Adding an accordion function to collapse different categories of owned object.
  • Showing the hierarchy of ownership, e.g. what objects are owned by the owned objects. @oxade mentioned that changes were being made to the API to simplify the extraction of grandchildren data. Perhaps this issue should be approached when we have such backend data?

@apburnie
Copy link
Contributor Author

apburnie commented Apr 4, 2022

@bwann52 --> the approval of this PR will close #1185.

@apburnie apburnie mentioned this pull request Apr 4, 2022
Closed
Clay-Mysten
Clay-Mysten suggested changes Apr 4, 2022
View reviewed changes
Copy link
Contributor

@Clay-Mysten Clay-Mysten left a comment

Choose a reason for hiding this comment

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

Markdown looks good with fixes to one sentence.

explorer/client/README.md Outdated Show resolved Hide resolved
Clay-Mysten
Clay-Mysten approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@Clay-Mysten Clay-Mysten left a comment

Choose a reason for hiding this comment

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

Great catch, Andrew! Thank you!

Clay-Mysten
Clay-Mysten approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@Clay-Mysten Clay-Mysten left a comment

Choose a reason for hiding this comment

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

Thanks, Andrew!

@bholc646 bholc646 removed the explorer label Apr 6, 2022
@bwann52 bwann52 assigned bwann52 and apburnie and unassigned bwann52 Apr 6, 2022
stella3d
stella3d suggested changes Apr 6, 2022
View reviewed changes
Copy link
Contributor

@stella3d stella3d left a comment

Choose a reason for hiding this comment

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

Some minor changes to make but this looks great overall!

explorer/client/src/components/ownedobjects/OwnedObjects.tsx Outdated Show resolved Hide resolved
explorer/client/src/components/ownedobjects/OwnedObjects.tsx Outdated Show resolved Hide resolved
explorer/client/src/components/ownedobjects/OwnedObjects.tsx Outdated Show resolved Hide resolved
explorer/client/src/utils/internetapi/SuiRpcClient.ts Show resolved Hide resolved
explorer/client/src/utils/static/mock_data.json Show resolved Hide resolved
@apburnie
Copy link
Contributor Author

apburnie commented Apr 7, 2022

Following a Slack Discussion with Stella, I've also removed the placeholder blank space for Preview Cards with no text. This and some chiselling at the padding spaces has enabled 3 Preview Cards per row. This means that on a Desktop Computer, when viewing Preview Cards with no text, you can see 12 Preview Cards on the one screen.

So on a reasonably small Surface Pro screen, you'll see:

image

On a 24" display, you'll see:

image

@apburnie apburnie requested a review from stella3d April 7, 2022 14:21
apburnie added 17 commits April 14, 2022 10:54
@apburnie
prevents unnecessary addition of page when exact division
684f80f
@apburnie
tidyup and changes to enable GitHub checks
1cde27e
@apburnie
removes Page 0 references and adds version number to preview
a84f355
@apburnie
fail state removed when display changed
700bc18
@apburnie
refactors tests
40dad3e
@apburnie
improves Object Results image display
7f8ebf1
@apburnie
improves image waiting style
70761ae
@apburnie
resolves label / ownedobject collision
b58fb65
@apburnie
updates README
99cce51
@apburnie
further updates to README
7f027c8
@apburnie
changes Owned Object API fetching component label
0f67213
@apburnie
relabels internetapi directory api
3bc4e0b
@apburnie
relabels InternetAPI API throughout components
6bba4d4
@apburnie
increased number of results from 4 per page to 12 per page and update…
cbde79c 
…d tests
@apburnie
enables use of generic types with SuiObject and ObjectInfoResponse
672b515
@apburnie
reduce preview card height
e76a399
@apburnie
switches to 3 items per row
d51544d
@apburnie apburnie force-pushed the explorer-previewcards branch from 725c215 to d51544d Compare April 14, 2022 10:06
@apburnie apburnie requested a review from Clay-Mysten April 14, 2022 10:38
Clay-Mysten
Clay-Mysten reviewed Apr 14, 2022
View reviewed changes
Copy link
Contributor

@Clay-Mysten Clay-Mysten left a comment

Choose a reason for hiding this comment

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

Do make sure you are coordinating with Stella as I see her iterating on similar text in #1269

@Clay-Mysten
Copy link
Contributor

Do make sure you are coordinating with Stella as I see her iterating on similar text in #1269

Ah, different files. Please disregard. Will re-review.

Clay-Mysten
Clay-Mysten approved these changes Apr 14, 2022
View reviewed changes
Copy link
Contributor

@Clay-Mysten Clay-Mysten left a comment

Choose a reason for hiding this comment

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

Thanks, Andrew!

@apburnie apburnie mentioned this pull request Apr 21, 2022
Closed
@stella3d stella3d merged commit c104597 into main Apr 21, 2022
@stella3d stella3d deleted the explorer-previewcards branch April 21, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Clay-Mysten Clay-Mysten Clay-Mysten approved these changes

@666lcz 666lcz 666lcz approved these changes

@stella3d stella3d stella3d approved these changes

@arun-koshy arun-koshy Awaiting requested review from arun-koshy

@oxade oxade Awaiting requested review from oxade

Assignees

@apburnie apburnie

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@apburnie @Clay-Mysten @stella3d @666lcz @bwann52 @bholc646