sui
sui copied to clipboard
[explorer] Image Pop Out
Adds functionality such that the image becomes larger when clicked.
Here's a video showing the functionality in action:
https://user-images.githubusercontent.com/11377188/181027342-98790126-be1e-4a28-a560-f8fe70331897.mp4
@Andrew47 a few pieces of feedback
- Could we keep the metadata text always aligned to the left edge of the image? Do you foresee any issue with that?
- The metadata text style - size/color - seems off. Please double check that.
- Is the overlay background color/opacity to spec? Seems a bit lighter than expected. May be it is just the video i am seeing.
@Andrew47 a few pieces of feedback
1. Could we keep the metadata text always aligned to the left edge of the image? Do you foresee any issue with that? 2. The metadata text style - size/color - seems off. Please double check that. 3. Is the overlay background color/opacity to spec? Seems a bit lighter than expected. May be it is just the video i am seeing.
Thank you for the feedback:
- The text appears more to the left than usual because this is a particularly narrow image. This looks reasonable with the wider image used in the live API. Unfortunately the live API service was down at the time this PR was made.
2/3. Yes - this follows the Figma design.
Got it. Okay.
I have a few thoughts on how we could improve this experience in future. Design needs to provide all the breakpoint specific layouts.
For the current PR, things look okay. I would as usual like to play with the UI/UX myself to provide further feedback post merge. Kindly tag me when its ready.
@Jordan-Mysten / @mystie711 / @stella3d / @Jibz1 / @666lcz --> This PR is now ready for re-review. Note that substantial changes have been implemented to add the Preview Media
button that also triggers the image to pop-out. The revamp also incorporates feedback from @Jordan-Mysten and @mystie711. This includes getting the text to align to the left of the image.
Figma with comments on what could be improved, https://www.figma.com/file/YJt9Bs4eyhD9PmeZQbFTkf/dev-feedback?node-id=0%3A1
Thank you @stella3d for the above feedback. I've updated the comments to better explain how Full Screen mode in the component is connected with Full Screen mode outside the component.
Hi team,
I suggest eventually developing some user docs for the Explorer. The closest we have now is in the Devnet docs: https://docs.sui.io/devnet/explore/devnet#mint-an-example-nft
With this page: https://docs.sui.io/devnet/explore#sui-explorer
Instead linking to our README: https://github.com/MystenLabs/sui/tree/main/explorer/client#readme
We might then reciprocally link from the Explorer to those docs. Just floating the idea here.
Hi team,
I suggest eventually developing some user docs for the Explorer. The closest we have now is in the Devnet docs: https://docs.sui.io/devnet/explore/devnet#mint-an-example-nft
With this page: https://docs.sui.io/devnet/explore#sui-explorer
Instead linking to our README: https://github.com/MystenLabs/sui/tree/main/explorer/client#readme
We might then reciprocally link from the Explorer to those docs. Just floating the idea here.
Good idea - this is not the PR to discuss this. Perhaps make a GitHub issue with more details on: who will use the docs, where should the docs be and what should the docs cover. This content either goes into the Explorer README or the Docs Portal.
Hi team, I suggest eventually developing some user docs for the Explorer. The closest we have now is in the Devnet docs: https://docs.sui.io/devnet/explore/devnet#mint-an-example-nft With this page: https://docs.sui.io/devnet/explore#sui-explorer Instead linking to our README: https://github.com/MystenLabs/sui/tree/main/explorer/client#readme We might then reciprocally link from the Explorer to those docs. Just floating the idea here.
Good idea - this is not the PR to discuss this. Perhaps make a GitHub issue with more details on: who will use the docs, where should the docs be and what should the docs cover. This content either goes into the Explorer README or the Docs Portal.
Good point. Doc issue filed here: https://github.com/MystenLabs/sui/issues/3899