🐛 show real image ratio in gallery redesign #4649

petersopko · 2023-01-09T19:39:29Z

Thank you for your contribution to the KodaDot NFT gallery.

👇 _ Let's make a quick check before the contribution.

PR Type

Bugfix
Feature
Refactoring

Context

Closes Mobile design on redesigned item #4647
Requires deployment <>

Before submitting pull request, please make sure:

My contribution builds clean without any errors or warnings
I've merged recent default branch -- main and I've no conflicts
I've tried to respect high code quality standards
I've didn't break any original functionality
I've posted a screenshot of demonstrated change in this PR

Optional

I've tested it at </bsx/collection>
I've tested PR on mobile
I've written unit tests 🧪
I've found edge cases

Had issue bounty label?

Fill up your KSM address: Payout

Community participation

Are you at KodaDot Discord?

Screenshot 📸

My fix has changed something on UI; a screenshot is best to understand changes for others.

netlify · 2023-01-09T19:39:33Z

✅ Deploy Preview for koda-nuxt ready!

Name	Link
🔨 Latest commit	`b6f7e1a`
🔍 Latest deploy log	https://app.netlify.com/sites/koda-nuxt/deploys/63c685fc5086be00087cc4b1
😎 Deploy Preview	https://deploy-preview-4649--koda-nuxt.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

petersopko · 2023-01-09T19:42:15Z

@exezbcz, there isn't a design for wide images in Figma; how do we want to handle it? Is this way okay?

exezbcz · 2023-01-09T20:28:06Z

for mobile it looks ok but on desktop it destroys the layout. I was thinking about making the img fit(width/height) to the rectangle. So we eliminate changing the layout.
like this:

Is that okay?

petersopko · 2023-01-10T09:00:07Z

I didn't want to add the event listener, so it won't resize responsively (you need to refresh between mobile and desktop), but that's usually only possible with devtools or if somebody wants to make their screen super small. I think it's okay, but maybe @roiLeo can take a look.

Also, I don't know if artists creating full-width pictures will be happy to have those images fitted inside a square.

roiLeo

tested on /rmrk/gallery/13528762-f091ff4a3f410ee872-ROXYNFT_FUN_BANNERS-FUN_BANNER_030-0000000000000030?redesign=true

With this change many items will now have "black square" frame, I'm not sure this is the best option

ex:
/rmrk/gallery/10371533-b6c51bd8c8868edc70-6ML5P-KUSAMA_WONDERLAND_037_OR_OCCUPIED-0000000000000043?redesign=true

or /snek/gallery/185770742-2?redesign=true

exezbcz · 2023-01-10T19:07:27Z

hmm, my bad.

I thought that like 90% are square images and different ratios are quite rare. Not sure what is the best solution for this.
Adding bars for the full widht img is not that bad but it does not work for imgs that are portrait oriented.

Possible solution is add a full screen button to the top right button section? Then it would be ok if we scale-crop the art to gallery img box.

Most nft marketplaces are designed for square content.

Again, what are your thoughts? Also curious about the data, how many images are not square?

some screens of how the major marketplaces deal with portrait content:

roiLeo · 2023-01-11T09:50:57Z

Possible solution is add a full screen button to the top right button section? Then it would be ok if we scale-crop the art to gallery img box.

In #4593 you've added it under the img box
Or we can add it in the top right like on OS

Again, what are your thoughts?

I would be in favour of removing the black stripes
First solution proposed by peter might be good, but we should still check if it doesn't disturb display of "square" images

Also curious about the data, how many images are not square?

I don't think you can tell, but you have to assume that any image size can be added

vikiival · 2023-01-11T11:57:37Z

libs/ui/src/components/MediaItem/type/ImageMedia.vue

-figure > img.image-media__image {
-  object-fit: cover;
+const isMobile = ref(window.innerWidth < 768)
+</script>


missing new line

vikiival · 2023-01-11T11:58:11Z

libs/ui/src/components/MediaItem/type/ImageMedia.vue

-  object-fit: cover;
+const isMobile = ref(window.innerWidth < 768)
+</script>
+<style>


missing scoped or better targeting of image

petersopko · 2023-01-11T14:42:15Z

I would be in favour of removing the black stripes
First solution proposed by peter might be good, but we should still check if it doesn't disturb display of "square" images

So are we going with this version? Can somebody confirm what should the final outcome here look like?

exezbcz · 2023-01-11T16:00:05Z

Possible solution is add a full screen button to the top right button section? Then it would be ok if we scale-crop the art to gallery img box.

In #4593 you've added it under the img box Or we can add it in the top right like on OS

yup, that was only quick idea and i posted it. Nothing rock solid imo :D

Again, what are your thoughts?

I would be in favour of removing the black stripes First solution proposed by peter might be good, but we should still check if it doesn't disturb display of "square" images

Well, its not that bad removing the stripes, not against it. What if we add the fullscreen button before production release?
I might be wrong, but i think the entire nft experience is still tailored to squares. Possible adding "art" view in the future? Will switch your layout and make the nft fit top part without cropping.

Also curious about the data, how many images are not square?

I don't think you can tell, but you have to assume that any image size can be added

yup

Sooo, lets handle it like on the nft cards?

petersopko · 2023-01-13T15:02:00Z

So are we going with this version? Can somebody confirm what should the final outcome here look like?

I'm sorry if I'm misunderstanding here, but can somebody tell me what should be done here to finish up?

I don't want to stall, but I didn't get the sense the discussion was over.

roiLeo · 2023-01-13T15:39:56Z

So are we going with this version? Can somebody confirm what should the final outcome here look like?

I'm sorry if I'm misunderstanding here, but can somebody tell me what should be done here to finish up?

I don't want to stall, but I didn't get the sense the discussion was over.

before

after

if it's ok for @exezbcz it's ok for me but you still got some small code changes request

petersopko · 2023-01-13T16:47:37Z

updated 👍🏻

roiLeo

✅ code lgtm

libs/ui/src/components/MediaItem/type/ImageMedia.vue

Co-authored-by: roiLeo <medina.leo42@gmail.com>

codeclimate · 2023-01-17T11:27:16Z

Code Climate has analyzed commit b6f7e1a and detected 0 issues on this pull request.

View more on Code Climate.

vikiival · 2023-01-17T12:58:27Z

pay 40 usd

yangwao · 2023-01-17T12:58:40Z

😍 Perfect, I’ve sent the payout
💵 $40 @ 35.36 USD/KSM ~ 1.131 $KSM
🧗 FNyt7T1xdbhN7dagf7yGYCRJuE3R45VmTCE3tjzy1rKxa7y
🔗 0x92ba0920a80f39c39c1d9a655e0e7badb10adce4e06e78af9672dcb3478ab211

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

petersopko · 2023-01-18T06:48:08Z

🔗 0x92ba0920a80f39c39c1d9a655e0e7badb10adce4e06e78af9672dcb3478ab211

👀

vikiival · 2023-01-18T08:16:57Z

Mr @yangwao was notified

yangwao · 2023-01-18T09:51:08Z

me & my few notifications

yup registered, will proceed

yangwao · 2023-01-18T12:09:16Z

pay 40

yangwao · 2023-01-18T12:09:28Z

😍 Perfect, I’ve sent the payout
💵 $40 @ 34.98 USD/KSM ~ 1.144 $KSM
🧗 FNyt7T1xdbhN7dagf7yGYCRJuE3R45VmTCE3tjzy1rKxa7y
🔗 0x51c8e07efd4b16f237365e2ce513bf5262a302edfc3b074c5c60f598e517e456

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

🔥 remove square limitation

6242ca0

petersopko requested a review from a team as a code owner January 9, 2023 19:39

petersopko requested review from prachi00 and removed request for a team January 9, 2023 19:39

💄 contain on desktop, no square on mobile

79a955c

roiLeo self-requested a review January 10, 2023 09:04

roiLeo reviewed Jan 10, 2023

View reviewed changes

yangwao requested a review from vikiival January 11, 2023 09:43

yangwao mentioned this pull request Jan 11, 2023

Launching Gallery item detail #4658

Closed

vikiival requested changes Jan 11, 2023

View reviewed changes

petersopko added 3 commits January 13, 2023 17:44

🎨 formatting + targetted style

54e5cfb

Merge branch 'main' into fix/4647-mobile-redesigned-item

a6d0728

Merge branch 'main' into fix/4647-mobile-redesigned-item

c6f8f5d

roiLeo approved these changes Jan 16, 2023

View reviewed changes

libs/ui/src/components/MediaItem/type/ImageMedia.vue Outdated Show resolved Hide resolved

petersopko and others added 2 commits January 16, 2023 11:49

Update libs/ui/src/components/MediaItem/type/ImageMedia.vue

b1af489

Co-authored-by: roiLeo <medina.leo42@gmail.com>

Merge branch 'main' into fix/4647-mobile-redesigned-item

13a1389

roiLeo added the S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved label Jan 16, 2023

vikiival approved these changes Jan 16, 2023

View reviewed changes

Merge branch 'main' into fix/4647-mobile-redesigned-item

5bf0b2a

vikiival enabled auto-merge January 16, 2023 12:14

Merge branch 'main' into fix/4647-mobile-redesigned-item

b6f7e1a

vikiival disabled auto-merge January 17, 2023 12:58

vikiival merged commit fa55f00 into kodadot:main Jan 17, 2023

yangwao added the paid pull-request has been paid label Jan 17, 2023

yangwao mentioned this pull request Jan 18, 2023

revisit failed payouts on pull requests #4748

Closed

yangwao removed the paid pull-request has been paid label Jan 18, 2023

yangwao added the paid pull-request has been paid label Jan 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 show real image ratio in gallery redesign #4649

🐛 show real image ratio in gallery redesign #4649

petersopko commented Jan 9, 2023 •

edited

netlify bot commented Jan 9, 2023 •

edited

petersopko commented Jan 9, 2023

exezbcz commented Jan 9, 2023

petersopko commented Jan 10, 2023

roiLeo left a comment

exezbcz commented Jan 10, 2023 •

edited

roiLeo commented Jan 11, 2023

vikiival Jan 11, 2023

vikiival Jan 11, 2023

petersopko commented Jan 11, 2023

exezbcz commented Jan 11, 2023

petersopko commented Jan 13, 2023

roiLeo commented Jan 13, 2023

petersopko commented Jan 13, 2023

roiLeo left a comment

codeclimate bot commented Jan 17, 2023

vikiival commented Jan 17, 2023

yangwao commented Jan 17, 2023

petersopko commented Jan 18, 2023

vikiival commented Jan 18, 2023

yangwao commented Jan 18, 2023

yangwao commented Jan 18, 2023

yangwao commented Jan 18, 2023

🐛 show real image ratio in gallery redesign #4649

🐛 show real image ratio in gallery redesign #4649

Conversation

petersopko commented Jan 9, 2023 • edited

PR Type

Context

Before submitting pull request, please make sure:

Optional

Had issue bounty label?

Community participation

Screenshot 📸

netlify bot commented Jan 9, 2023 • edited

✅ Deploy Preview for koda-nuxt ready!

petersopko commented Jan 9, 2023

exezbcz commented Jan 9, 2023

petersopko commented Jan 10, 2023

roiLeo left a comment

Choose a reason for hiding this comment

exezbcz commented Jan 10, 2023 • edited

roiLeo commented Jan 11, 2023

vikiival Jan 11, 2023

Choose a reason for hiding this comment

vikiival Jan 11, 2023

Choose a reason for hiding this comment

petersopko commented Jan 11, 2023

exezbcz commented Jan 11, 2023

petersopko commented Jan 13, 2023

roiLeo commented Jan 13, 2023

petersopko commented Jan 13, 2023

roiLeo left a comment

Choose a reason for hiding this comment

codeclimate bot commented Jan 17, 2023

vikiival commented Jan 17, 2023

yangwao commented Jan 17, 2023

petersopko commented Jan 18, 2023

vikiival commented Jan 18, 2023

yangwao commented Jan 18, 2023

yangwao commented Jan 18, 2023

yangwao commented Jan 18, 2023

petersopko commented Jan 9, 2023 •

edited

netlify bot commented Jan 9, 2023 •

edited

exezbcz commented Jan 10, 2023 •

edited