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

feat: NFT details panel #476

Merged
merged 16 commits into from
Sep 4, 2023
Merged

feat: NFT details panel #476

merged 16 commits into from
Sep 4, 2023

Conversation

janmichek
Copy link
Collaborator

@janmichek janmichek commented Aug 24, 2023 •
edited

Description

resolves #145

Demo

firefox_TANskxZ4Hk.mp4

Checklist:

@github-actions
Copy link

Deployed to https://pr-476-aescan.stg.aepps.com

janmichek
janmichek commented Aug 28, 2023
View reviewed changes
src/pages/keyblocks/[id].vue
@@ -13,7 +13,6 @@

<template v-if="!isLoading">
<keyblock-details-panel
v-if="keyblockDetails"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cleanup - handled by loader

michele-franchi
michele-franchi requested changes Sep 1, 2023
View reviewed changes
Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

Good job, I added some suggestions.

src/utils/hints/nftHints.js Outdated Show resolved Hide resolved
src/utils/format.js Outdated Show resolved Hide resolved
src/utils/format.js Show resolved Hide resolved
lukeromanowicz
lukeromanowicz approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@lukeromanowicz lukeromanowicz left a comment •
edited

Choose a reason for hiding this comment

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

Very decent job 💪 There is one detail missing and it could use eslint rerun but I approve in advance

src/pages/nfts/[id].vue Show resolved Hide resolved
michele-franchi
michele-franchi requested changes Sep 4, 2023
View reviewed changes
Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

I added some suggestions, some hints are not aligned to the NFTs list PR.

On mobile the Collection Name row goes to two lines, is it possible to fix it?
image

src/utils/hints/nftHints.js Outdated Show resolved Hide resolved
src/utils/hints/nftHints.js Outdated Show resolved Hide resolved
src/components/NftDetailsPanel.vue Outdated Show resolved Hide resolved
janmichek and others added 16 commits September 4, 2023 10:58
@janmichek
print plain values to the panel
ca6141b
@janmichek
add limit logic
45e1ff7
@janmichek
add hints
e773afe
@janmichek
adapt data in adapter
dc0c742
@janmichek
fix initialization component
7ea4c6b
@janmichek
move formatting
bf6746e
@janmichek
change loading condition
294b6bb
@janmichek @michele-franchi
Update src/utils/hints/nftHints.js
af5a50b 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek @michele-franchi
Update src/utils/format.js
8b9be50 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek @michele-franchi
Update src/utils/format.js
07b7cf1 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek
fix code style
7f8f412
@janmichek
add error throw
8e5a160
@janmichek @michele-franchi
Update src/utils/hints/nftHints.js
920d763 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek @michele-franchi
Update src/utils/hints/nftHints.js
2f4a1fc 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek @michele-franchi
Update src/components/NftDetailsPanel.vue
01adfbd 
Co-authored-by: Michele F. <michele-franchi@users.noreply.github.com>
@janmichek
fixes after rebase
b907c19
janmichek
janmichek commented Sep 4, 2023
View reviewed changes
src/components/TokenDetailsPanel.vue
@@ -236,11 +234,6 @@ const marketCap = computed(() =>
}
}

&__chip {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

clenup

janmichek
janmichek commented Sep 4, 2023
View reviewed changes
src/utils/adapters.js

function isAuction(name) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cleanup

@janmichek
Copy link
Collaborator Author

I added some suggestions, some hints are not aligned to the NFTs list PR.

Now it is rebased

@janmichek
Copy link
Collaborator Author

On mobile the Collection Name row goes to two lines, is it possible to fix it?

At the exact resolution I have differently broken content, but anyway, it's a more general issue. I am not really sure how to fix it in elegant way ./

michele-franchi
michele-franchi approved these changes Sep 4, 2023
View reviewed changes
Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

Looks good 👏

@CharisSiarampalis
Copy link

@janmichek Is the issue with the collection name going to get fixed on this pr?
Because i tried different mobile resolutions and it always appear on that spot
image

@janmichek janmichek mentioned this pull request Sep 4, 2023
Open
@janmichek
Copy link
Collaborator Author

@janmichek Is the issue with the collection name going to get fixed on this pr? Because i tried different mobile resolutions and it always appear on that spot image

Created a followup #493

@janmichek janmichek merged commit c3fa902 into develop Sep 4, 2023
2 checks passed
@janmichek janmichek deleted the NFT-details-panel branch September 4, 2023 12:08
@aeternity-bot aeternity-bot mentioned this pull request Sep 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michele-franchi michele-franchi michele-franchi approved these changes

@CharisSiarampalis CharisSiarampalis CharisSiarampalis approved these changes

@lukeromanowicz lukeromanowicz lukeromanowicz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NFT details panel
4 participants
@janmichek @CharisSiarampalis @michele-franchi @lukeromanowicz