Allow to open viewer in a separate window #548

pandablue0809 · 2025-08-19T13:28:35Z

Issue

Make possible to open the integrated viewer in a separate window (/nft page).

ihomp · 2025-09-14T00:02:06Z

@Anna15170221 can you test this one and write your suggestions?

from me: it seems a lot of code was copy pasted from a different component, we shouldn't copy-paste the code, we need to extract it and reuse it.. that way we will keep the code maintanable.

the full-screen view - has trouble with scrolling; also, the scrolling button shouldn't be there.
the buttons to view full screen and to download music should be in the same style (new style you suggested) ..
when we have both audio and image - we have two buttons to view full screen - that shoudn't be the case.

ihomp · 2025-09-14T00:03:24Z

why search block css was changed? that can break with some popups scrolling for fixed objects etc.. that is need to be tested well.

ihomp

It seems there is a lot of copy-pasted code that needs to be extracted and reused.

Anna15170221 · 2025-09-15T08:35:36Z

@pandablue0809

the buttons are on the image - for me doesn't look nice for desktop
and the mobile view should also be improved I think

http://localhost:3000/en/nft/00083A9836EC5BC756007990B29DBFC81F45460ED8B8583A6442C30600000069 - I tested on this one

Anna15170221 · 2025-09-15T08:37:26Z

@ihomp Slava, popups scrolling for fixed objects - what do you mean here?
that can break with some popups scrolling for fixed objects etc.. that is need to be tested well.

pandablue0809 · 2025-09-17T19:29:50Z

@ihomp
i fixed it. plz review again. 🙏

ihomp · 2025-09-29T08:16:13Z

@ihomp Slava, popups scrolling for fixed objects - what do you mean here? that can break with some popups scrolling for fixed objects etc.. that is need to be tested well.

@pandablue0809 chenaged css for the search block, so the search block on other pages can appear over some popuup windows like choosinga. token or when signing transactions, or when scrolling on a pages, some elemnts can be under the bar or over the bar.. it's dangerous to change such parameters without proper testing on different pages with different scenarious

ihomp · 2025-09-29T08:16:42Z

@ihomp i fixed it. plz review again. 🙏

@Anna15170221 can you test again please

…-separate-window-on-nft-page

pandablue0809 · 2025-10-12T20:28:35Z

@ihomp
i fixed it.
plz review again. 🙏

Anna15170221 · 2025-10-13T09:30:26Z

@pandablue0809
the button "video" is on the image until you open the full screen size.

The rest looks ok for me.

pandablue0809 · 2025-10-13T16:29:11Z

@Anna15170221 @ihomp
i fixed it.
plz review again. 🙏

Anna15170221 · 2025-10-14T06:50:00Z

@pandablue0809 @ihomp
looks ok for me.

ihomp

There are a lot of copy-pasted functions and code.
You can not copy-paste the same function within the same app, they need to be extracted as functions and reused.

…-separate-window-on-nft-page

pandablue0809 · 2025-11-09T04:48:40Z

@ihomp
i fixed it
plz review again 🙏

ihomp

Please check how you used styles in the other pages you did:
/pages/nft-collection
/pages/objects
/pages/services/account-settings

Please use it here in the same way.

pandablue0809 · 2025-11-10T11:11:58Z

@ihomp
i fixed it
plz review again 🙏

ihomp · 2025-11-21T13:41:30Z

@Anna15170221 can you please test again, please

Anna15170221 · 2025-11-21T14:27:03Z

@ihomp for me looks ok.

components/NftPreview.js

ihomp · 2025-11-21T15:50:16Z

components/NftPreview.js

          )}
          {videoUrl && defaultTab === 'video' && (
            <>
-              {loadingImage(nft)}


why nft is deleted?

ihomp · 2025-11-21T15:58:25Z

There are many things affected by this PR:
I see some logic was deleted for loading states for models for example.

We need to test:
NFT pages with:

image + audio
image + video
image + audio + video
image + model
model
video

We need to make sure
When the model is loading, we see the loading state on the NFT page
When testing, we need to check how it is now and how it will be after the PR.

we need to make sure we have proper loading states

videos
images
models

We need to make sure we show errors for failing loading images
that 18+ still works on the nft page

@Anna15170221 @pandablue0809, please send me some examples where you've tested, so I can check too.

Anna15170221 · 2025-11-24T14:16:05Z

@ihomp
here are some examples I've tested on:
image+audio
http://localhost:3000/en/nft/00081F40DD3FF58441A1706F5FBD672B9F47AAB5D07D91FBFCF7C60405420369
image+video
http://localhost:3000/en/nft/00080000ED141596DF7FEE3ABF30440FDC9D8E9A1BF34A66122DCB5B000001C0
image+video+audio
http://localhost:3000/en/nft/00083A988FAD76FECCD5D023F7A2D7710CF6297C622492EE977719F105777A85
model
http://localhost:3000/en/nft/0008000025B89F24B381CABA5921FF0B634DE9111D915B2A656A947D000008E2

What I've noticed that in the "full screen mode" which Riku is trying to implement, when we open the video in the full screen, we show it in the bad quality.
Concerning the model in the "full screen mode" we show only the small size without the opportunity to show the full screen size.

I haven't found the nft with failed to load error.

ihomp · 2025-11-25T12:20:24Z

The code is still not optimized....

I see that you reuse the same CSS file in the NFTpreview...

That wouldn't be nessary if you reused components and functions instead of css classes..

For example:

loading image - you have just copy pasted my code to your file, and then reused the css in it..
this is wrong..

You shouldn't copy and paste code; instead, extract my code as a function and reuse it on both my page and your new page.

If files will have some shared CSS (which is not the part of the same components) then you should name the css file differently, like nftView.css or similiar.. importing NFTfullViewer css (css for another page) into the NftPreview - doesn't look right..

also css modules can extend one another..
for example you can have
css which is common for both pages, you can call it nftViewer - then you can import it on NftPreview page
and then you can ceate a new css file NftFullViewer that will IMPORT the css from nftViewer and extend it with new css specifically for that page, and then import NftFullViewer css to NftFullViewer page.

ihomp · 2025-11-25T12:25:14Z

components/NftPreview.js

-  const modelUrl = nftUrl(nft, 'model')
-  const viewerUrl = nftUrl(nft, 'viewer')
+  const urls = extractNftUrls(nft)
+  const { image: imageUrl, video: videoUrl, audio: audioUrl, model: modelUrl, viewer: viewerUrl } = urls


why , name it right away as it will be used.
Instead of reassigning teh variables, just update teh function

ihomp · 2025-11-25T12:26:11Z

components/NftPreview.js

    }
    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [imageUrl, videoUrl])
+  }, [imageUrl, videoUrl, nft.metadata])


any comments?

ihomp · 2025-11-25T12:26:41Z

components/NftPreview.js

+
  return (
-    <>
+    <div className={nftFullScreenViewer}>


If you want to reuse the same classes, your rather name the file differently

First of all, you should export components taht are reused, not classes

ihomp

JS/CSS code sharing is not optimised

components/NftPreview.js

ihomp · 2025-11-25T12:29:55Z

pages/nft/[[...id]].js

        image={{ file: imageUrl }}
      />
-      <SearchBlock searchPlaceholderText={t('enter-nft-id', { ns: 'nft' })} tab="nft" />
+      <div style={isHidden ? { position: 'relative', zIndex: 0 } : {}}>


Thats looks like a hack.. why is it this way?

…ps://github.com/pandablue0809/bithomp-frontend-react into feat/520/allow-open-separate-window-on-nft-page

pandablue0809 · 2025-11-26T15:21:32Z

@ihomp
i fixed it.
#548 (comment)
i need this, because there is asways search block in fullscreen viewer.
plz review again. 🙏

Copilot

Pull request overview

This PR implements a fullscreen viewer feature for NFTs, allowing users to view NFT content in a dedicated fullscreen mode. The changes refactor existing NFT preview functionality into reusable components and introduce new UI components for fullscreen viewing.

Key Changes:

Extracted NFT content rendering logic into reusable components and utility functions
Added a new fullscreen viewer component with mobile-responsive design
Created a custom hook (useNftContent) to manage NFT content state and logic

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
utils/nft.js	Added utility functions for NFT content handling (panorama detection, model attributes processing, URL extraction, rendering helpers)
styles/components/nftPreview.module.scss	New stylesheet for NFT preview component with shared styles from NftViewer.scss
styles/components/nftFullScreenViewer.module.scss	New stylesheet for fullscreen viewer with responsive mobile design
styles/components/NftViewer.scss	New shared SCSS mixin for common NFT viewer styles
pages/nft/[[...id]].js	Added state management for hiding content when fullscreen is active
hooks/useNftContent.js	New custom hook centralizing NFT content state management and URL extraction
components/NftPreview.js	Refactored to use new hook and components, added fullscreen button
components/NftFullScreenViewer.js	New fullscreen viewer component with tab navigation and download functionality
components/Nft/NftContentRenderer.js	New shared component for rendering NFT media content (images, videos, 3D models)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-04T10:44:05Z

hooks/useNftContent.js

+export const useNftContent = (nft, imageStyleOptions = {}) => {
+  const { t } = useTranslation()
+  const [contentTab, setContentTab] = useState('image')
+  const [loaded, setLoaded] = useState(true)


The initial value of loaded should be false instead of true. Setting it to true initially prevents loading indicators from showing when content is actually loading, which creates a confusing user experience.

Suggested change

const [loaded, setLoaded] = useState(true)

const [loaded, setLoaded] = useState(false)

Copilot · 2025-12-04T10:44:06Z

components/Nft/NftContentRenderer.js

+                  style={(() => {
+                    if (classNamePrefix === 'fv-fullscreen') {
+                      return {
+                        imageRendering: imageStyle.imageRendering,
+                        filter: imageStyle.filter,
+                        display: loaded ? 'inline-block' : 'none'
+                      }
+                    }
+                    return {
+                      ...imageStyle,
+                      display: loaded ? 'inline-block' : 'none'
+                    }
+                  })()}


The inline IIFE for computing styles is complex and reduces readability. Extract this logic into a named function or compute the style object before the return statement to improve code clarity.

add full screen viewr

c6d7658

pandablue0809 requested a review from ihomp August 19, 2025 13:28

ihomp requested changes Sep 14, 2025

View reviewed changes

ihomp added the Priority 4 label Sep 14, 2025

fix buttons and UIs

a54f918

pandablue0809 requested a review from ihomp September 17, 2025 19:30

ihomp requested a review from Anna15170221 September 29, 2025 08:16

pandablue0809 added 3 commits October 12, 2025 15:45

Merge remote-tracking branch 'upstream/main' into feat/520/allow-open…

a61e062

…-separate-window-on-nft-page

Merge remote-tracking branch 'upstream/main' into feat/520/allow-open…

585886d

…-separate-window-on-nft-page

remove searchblock changes

1226850

fix styles in fullscreenviewer

431c493

ihomp requested changes Oct 15, 2025

View reviewed changes

pandablue0809 added 2 commits November 9, 2025 13:34

optimize the code

2504efb

Merge remote-tracking branch 'upstream/main' into feat/520/allow-open…

f8ab9b1

…-separate-window-on-nft-page

pandablue0809 requested a review from ihomp November 9, 2025 04:48

ihomp requested changes Nov 10, 2025

View reviewed changes

fix using the styles

e707d07

pandablue0809 requested a review from ihomp November 10, 2025 11:13

Merge branch 'main' into pr/548

f095bcf

ihomp reviewed Nov 21, 2025

View reviewed changes

components/NftPreview.js Show resolved Hide resolved

ihomp reviewed Nov 21, 2025

View reviewed changes

components/NftPreview.js

)}

{videoUrl && defaultTab === 'video' && (

<>

{loadingImage(nft)}

Copy link

Member

ihomp Nov 21, 2025

Choose a reason for hiding this comment

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

why nft is deleted?

ihomp reviewed Nov 25, 2025

View reviewed changes

ihomp requested changes Nov 25, 2025

View reviewed changes

pandablue0809 added 3 commits November 26, 2025 23:58

optimised css and js code and add model loading state

3475a8d

Merge branch 'feat/520/allow-open-separate-window-on-nft-page' of htt…

b8a9031

…ps://github.com/pandablue0809/bithomp-frontend-react into feat/520/allow-open-separate-window-on-nft-page

add last line in nft.js

c89be0d

pandablue0809 requested a review from ihomp November 27, 2025 09:30

ihomp requested a review from Copilot December 4, 2025 10:41

Copilot started reviewing on behalf of ihomp December 4, 2025 10:42 View session

Copilot finished reviewing on behalf of ihomp December 4, 2025 10:42

Copilot AI reviewed Dec 4, 2025

View reviewed changes

	const [loaded, setLoaded] = useState(true)
	const [loaded, setLoaded] = useState(false)

Allow to open viewer in a separate window #548

Are you sure you want to change the base?

Allow to open viewer in a separate window #548

Conversation

pandablue0809 commented Aug 19, 2025

Issue

Uh oh!

ihomp commented Sep 14, 2025

Uh oh!

ihomp commented Sep 14, 2025

Uh oh!

ihomp left a comment

Choose a reason for hiding this comment

Uh oh!

Anna15170221 commented Sep 15, 2025

Uh oh!

Anna15170221 commented Sep 15, 2025

Uh oh!

pandablue0809 commented Sep 17, 2025

Uh oh!

ihomp commented Sep 29, 2025

Uh oh!

ihomp commented Sep 29, 2025

Uh oh!

pandablue0809 commented Oct 12, 2025

Uh oh!

Anna15170221 commented Oct 13, 2025

Uh oh!

pandablue0809 commented Oct 13, 2025

Uh oh!

Anna15170221 commented Oct 14, 2025

Uh oh!

ihomp left a comment

Choose a reason for hiding this comment

Uh oh!

pandablue0809 commented Nov 9, 2025

Uh oh!

ihomp left a comment

Choose a reason for hiding this comment

Uh oh!

pandablue0809 commented Nov 10, 2025

Uh oh!

ihomp commented Nov 21, 2025

Uh oh!

Anna15170221 commented Nov 21, 2025

Uh oh!

Uh oh!

ihomp Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

ihomp commented Nov 21, 2025

Uh oh!

Anna15170221 commented Nov 24, 2025

Uh oh!

ihomp commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ihomp Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

ihomp Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

ihomp Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

ihomp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ihomp Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

pandablue0809 commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ihomp commented Nov 25, 2025 •

edited

Loading