Skip to content

Conversation

@ddey2
Copy link
Member

@ddey2 ddey2 commented Sep 11, 2023

No description provided.

@ddey2 ddey2 added bug Something isn't working frontend Frontend specific (react) labels Sep 11, 2023
@ddey2 ddey2 requested a review from longshuicy as a code owner September 11, 2023 22:09
@ddey2 ddey2 linked an issue Sep 11, 2023 that may be closed by this pull request
@ddey2
Copy link
Member Author

ddey2 commented Sep 11, 2023

Screenshot 2023-09-11 at 5 09 51 PM Somehow it's rendering it 6times. I am looking into why/where this piece of code is getting called.

Copy link
Member

@longshuicy longshuicy left a comment

Choose a reason for hiding this comment

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

Functionality works. We might want to improve the visual in a subsequent task?

@ddey2
Copy link
Member Author

ddey2 commented Sep 13, 2023

Updated the ui. I think for now this should be good. We can alter it later if needed.
Screenshot 2023-09-13 at 12 24 06 PM

@max-zilla
Copy link
Contributor

Generally works as expected. However, if image files are small enough to use raw bytes but there is no viz config, it still shows the error message along with the raw preview which might be confusing:
image

I think we should only show this message if we can't do anything with the raw bytes. Not sure how easy that is to determine right now though (i.e. would we have to check MIME types and stuff?)

Copy link
Member

@longshuicy longshuicy left a comment

Choose a reason for hiding this comment

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

Like we discussed on Thursday, you might need to add similar logic in the useEffect.

Copy link
Contributor

@max-zilla max-zilla left a comment

Choose a reason for hiding this comment

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

Correctly shows when file is too big for bytes preview, when no viz config is available otherwise, and does not show either if a byte preview is shown. behaves as expected

Copy link
Member

@longshuicy longshuicy left a comment

Choose a reason for hiding this comment

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

Tested a few scenarios. Works and approve.

@longshuicy longshuicy merged commit 418baa2 into main Sep 19, 2023
@longshuicy longshuicy deleted the 713-dont-show-empty-page-for-visualization branch September 19, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend Frontend specific (react)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't show empty page for visualization

4 participants