Collection mobile

[daiagi]

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 Redesign Collection Mobile #5076
• 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.

[kodabot]

WARNING @daiagi PR for issue #5076 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #5076

[netlify]

✅ Deploy Preview for koda-nuxt ready!

Name	Link
🔨 Latest commit	583fc56
🔍 Latest deploy log	https://app.netlify.com/sites/koda-nuxt/deploys/6406e536c61f800008a3736a
😎 Deploy Preview	https://deploy-preview-5158--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.

[daiagi]

will require merge resolution with #5146
need to merge #5120 before merging this

@exezbcz ive hidden the display grid button for screen between mobile and desktop in order to avoid wrapping in that screen size range (around 780 px)
Is that OK?

[exezbcz]

@daiagi yup, that is correct, there is no grid button on mobile - mobile nft cards are also bit different

I will have a look 👍

[preschian]

will require merge resolution with #5146 need to merge #5120 before merging this

maybe set this as draft PR first? after those PR merged we can take a look again. and fix the DeepScan

[exezbcz]

yes, that was wrong on my end then :D

last changes:

• the whitespace
• remove buy now by default from collection

thanks!

[daiagi]

yes, that was wrong on my end then :D

last changes:

* the whitespace

* remove buy now by default from collection

thanks!

Done

[roiLeo]

Don't know if it's on mobile spec but I see stuff changed:

• creator link should be blue
  
• explore layout spacing issue (compare it with beta)
  
• weird animation on close sidebar filter

[roiLeo]

collectionStatsByIdWithOffers retrieve all data on client browser :/

[daiagi]

weird animation on close sidebar filter

exists on beta as well, should solve in separate issue

explore layout spacing issue (compare it with beta)

indeed, apologies. fixed

---

• Collection mobile #5158 (comment)

here is an example:
https://deploy-preview-5158--koda-nuxt.netlify.app/bsx/collection/3268254081?page=1&redesign=true

if i remove the has-text-link class from the internal <a> tag the text is black

[roiLeo]

if i remove the has-text-link class from the internal <a> tag the text is black

Can't reproduce it, we have same stuff going in GalleryItemDescription component so I want to unify how it looks

[daiagi]

if i remove the has-text-link class from the internal <a> tag the text is black

Can't reproduce it, we have same stuff going in GalleryItemDescription component so I want to unify how it looks

toggle mobile device in dev tools and make sure it is in "mobile" mode aka touch enabled. refresh page

from IdentityLink.vue

maybe remove this !isMobileDevice => render identityPopover on touch devices as well ? wtyt @roiLeo

[roiLeo]

toggle mobile device in dev tools and make sure it is in "mobile" mode aka touch enabled. refresh page

Aight, I can reproduce it with mobile mode.

from IdentityLink.vue

This is where issue come from, since IdentityLink provide a nested <a/> tag, IMO we should replace this component by something else since when you change a tag to div it work like a charm

maybe remove this !isMobileDevice => render identityPopover on touch devices as well ? wtyt @roiLeo

yep easy solution, but I think this has been introduce for a particular reason

[daiagi]

Mind approving this PR and I'll open a new issue to deal with this?

[roiLeo]

What need to be improved:

• profile link on mobile (IdentityLink component)
• use NeoSidebar instead of Buefy Sidebar (MobileFilter component)
• index folder in /components/explore ???
• remove linkClass prop
• remove class="mb-5" (total in ItemsGrid & Gallery component)
• change position: relative with is-relative helper
• v-if on ActiveCount usage

otherwise lgtm

[codeclimate]

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

View more on Code Climate.

[daiagi]

What need to be improved:

• profile link on mobile (IdentityLink component)
• use NeoSidebar instead of Buefy Sidebar (MobileFilter component)
• index folder in /components/explore ???
• remove linkClass prop
• remove class="mb-5" (total in ItemsGrid & Gallery component)
• change position: relative with is-relative helper
• v-if on ActiveCount usage

All done except:

Leftovers

• profile link on mobile (IdentityLink component)
• remove linkClass prop

• allow straightforward styling of IdentityLink #5188

[daiagi]

@Jarsen136 @preschian could i have your review please?

[Jarsen136]

code lgtm