sui icon indicating copy to clipboard operation
sui copied to clipboard

[explorer] Image Pop Out

Open apburnie opened this issue 2 years ago • 3 comments

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

apburnie avatar Jul 19 '22 15:07 apburnie

@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.

mystie711 avatar Jul 26 '22 14:07 mystie711

@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:

  1. 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.

apburnie avatar Jul 26 '22 14:07 apburnie

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.

mystie711 avatar Jul 26 '22 14:07 mystie711

@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.

apburnie avatar Aug 04 '22 09:08 apburnie

Figma with comments on what could be improved, https://www.figma.com/file/YJt9Bs4eyhD9PmeZQbFTkf/dev-feedback?node-id=0%3A1

mystie711 avatar Aug 04 '22 16:08 mystie711

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.

apburnie avatar Aug 08 '22 09:08 apburnie

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.

Clay-Mysten avatar Aug 09 '22 17:08 Clay-Mysten

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.

apburnie avatar Aug 10 '22 08:08 apburnie

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

Clay-Mysten avatar Aug 10 '22 17:08 Clay-Mysten