#4555 explore route changes #4577

prachi00 · 2022-12-28T22:40:56Z

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 Explorer Tabs bar #4555
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 · 2022-12-28T22:41:00Z

✅ Deploy Preview for koda-nuxt ready!

Name	Link
🔨 Latest commit	`b7570bd`
🔍 Latest deploy log	https://app.netlify.com/sites/koda-nuxt/deploys/63c0d6902bfc360009d30d76
😎 Deploy Preview	https://deploy-preview-4577--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.

kodabot · 2022-12-28T22:41:00Z

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

prachi00 · 2022-12-28T22:48:16Z

Figma: https://www.figma.com/file/OwAEyE0QDPn3tHXZbyot1o/Explore-v0.9?node-id=1%3A493&t=lvgyGixqOvZVjmmn-0

yangwao · 2023-01-01T19:50:39Z

https://deploy-preview-4577--koda-nuxt.netlify.app/bsx/explore/items?page=1

Seems this lines doesn't happen on main branch

roiLeo

2 things:

should we keep prefix in path? (/${prefix}/explore/...) I thought that it will be chainless
I don't see much difference in naming items vs collectibles for me it mean the same thing. Can't we have something else?

components/rmrk/ExploreLayout/ExploreLayout.vue

pages/_prefix/explore/items.vue

components/rmrk/ExploreLayout/ExploreLayout.vue

yangwao · 2023-01-03T17:31:15Z

It's possible to remove that blackout effect and render the component somehow without during switching tabs?

Screen.Recording.2023-01-03.at.18.29.43.mov

prachi00 · 2023-01-04T00:53:13Z

It's possible to remove that blackout effect and render the component somehow without during switching tabs?

Screen.Recording.2023-01-03.at.18.29.43.mov

done

components/rmrk/ExploreLayout/ExploreLayout.vue

yangwao · 2023-01-04T12:19:48Z

Oh missed this one.

should we keep prefix in path? (/${prefix}/explore/...) I thought that it will be chainless

Right now, yes, keep it and another PR we will remove another as supabase endpoint is available, as I'm not sure if we will miss something or so, just one thing at time 🙏 😭

Enable Multi-chain capabilities in KodaDot #4257

I don't see much difference in naming items vs collectibles for me it mean the same thing.

Let me visit library, I guess it depends on context, as more likely with artist items I would stick with collectibles, but having collectibles next to the collection sounds similar and visually not that distinguishing.

So collections/items would be ok for now if we don't come something, could be separate issue later.

https://www.dictionary.com/browse/collectibles

https://www.dictionary.com/browse/item

Can't we have something else?

https://www.thesaurus.com/browse/collectibles

yangwao · 2023-01-04T12:22:43Z

done

Seems black transition is back on second time?
Tab states are not reflecting where the user wants to go and return me back?

Screen.Recording.2023-01-04.at.13.20.23.mov

Let's make it "Collections" and "Items" for now :)
Plus the tick symbol missing?

design https://user-images.githubusercontent.com/5887929/209315319-d2cd5a7e-c7db-49b4-852c-d28ce584e243.png

roiLeo · 2023-01-04T13:36:27Z

Hey!
How to distinguish filters by chain?

example:

Most Emoted on rmrk
Most Offers on bsx

yangwao · 2023-01-04T15:23:47Z

How to distinguish filters by chain?

I heard whispers in cave something about supabase @vikiival

We can disable them till now:)

roiLeo · 2023-01-04T15:40:56Z

I heard whispers in cave something about supabase @vikiival

please, enlight us!

prachi00 · 2023-01-05T03:34:47Z

does anyone know how to pass v-model as a prop in vue3, need it for the coomon tab component in libs, but unable to make it work, which is causing issues @roiLeo @preschian @Jarsen136

prachi00 · 2023-01-05T03:40:51Z

To explain more:

we have this in explore component, I want the v-model to be passed to NeoTab and in turn call the getter and setter when it changes, but it is not happening

vikiival

/explore throws 404

https://deploy-preview-4577--koda-nuxt.netlify.app/bsx/explore

components/shared/exploreTabsFilterSort/ExploreTabsFilterSort.vue

vikiival · 2023-01-11T12:11:33Z

components/shared/exploreTabsFilterSort/ExploreTabsFilterSort.vue

+}
+</script>
+
+<style lang="scss">


missing scoped?

prachi00 marked this conversation as resolved.

components/shared/exploreTabsFilterSort/ExploreTabsFilterSort.vue

vikiival · 2023-01-11T12:12:08Z

components/shared/exploreTabsFilterSort/ExploreTabsFilterSort.vue

+.explore-tabs {
+  .btn-items {
+    width: 240px;
+    left: -6px;


I dont think there is any way to make the two buttons meet without any negative margin, since we are trying to make them look like tabs

I dont think there is any way to make the two buttons meet without any negative margin,

bulma doc, buefy doc

since we are trying to make them look like tabs

why don't we use tab?

it wasn't working properly, so we decided to use buttons
CC @yangwao @preschian

yes, this is because we are using a new layout. usually, the UI framework provides a button group. currently, oruga doesn't have that

https://mui.com/material-ui/react-button-group/

https://chakra-ui.com/docs/components/button#grouping-buttons

https://tailwindui.com/components/application-ui/elements/button-groups

I dont think there is any way to make the two buttons meet without any negative margin

@prachi00 maybe try with the bulma approach that @roiLeo sent the link

TIL #4577 (comment), just read this message

removed the negative margin

layouts/explore-layout.vue

roiLeo

Hey! why did you create a specific layout? I don't think it's needed from what I can see

prachi00 · 2023-01-12T08:22:08Z

Hey! why did you create a specific layout? I don't think it's needed from what I can see

@roiLeo so this was the approach I followed #4577 (comment)

preschian · 2023-01-12T08:24:38Z

Hey! why did you create a specific layout? I don't think it's needed from what I can see

That's me the one who suggests using nested layouts. it should be easier if we use nested layouts in this case because the filter/sort button will be unified in the next iterate design. IMO profile/collection page should be using nested layouts also instead of tabs

this is kinda similar to this https://beta.nextjs.org/docs/routing/pages-and-layouts#nesting-layouts

roiLeo · 2023-01-12T08:35:36Z

That's me the one who suggests using nested layouts. it should be easier if we use nested layouts in this case because the filter/sort button will be unified in the next iterate design.

I still don't understand the purpose but let's see.
Remember that we'll have different sorting option from chain

IMO profile/collection page should be using nested layouts also instead of tabs

so we'll create another layout?

preschian · 2023-01-12T08:59:10Z

I still don't understand the purpose but let's see.
Remember that we'll have different sorting option from chain
so we'll create another layout?

yes, IMO better to use a nested layout. please take a look at the attached image or this link https://nextjs.org/blog/layouts-rfc for the purposes

in this picture, <Header />, <Footer />, and <DashboardSidebar /> are on the layout. in our case <DashboardSidebar /> === <SortFilter />. why did we put that component in the layout? because when we navigate to Collectibles/Item the content only changes on the dashed area (see the picture)

before next.js 13 support nested routes, usually I create multiple layouts to deliver the same goals with that https://constantsolutions.dk/2020/02/nested-layouts-in-nuxt-vue-js/

roiLeo · 2023-01-12T09:10:17Z

ok thanks for explenation I understand better we will use tabs as breadcrumb for explore pages

codeclimate · 2023-01-13T03:57:27Z

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

View more on Code Climate.

vikiival

Haven't tested, code looks LGTM

roiLeo

✅ it looks good!

Collection

Gallery

But mobile is not very optimal yet

We can expand this in another issue

Note

mobile design need some love
border changes on pagination is done on purpuse?
find a bug when on /rmrk/explore/items & click "Collections" in navbar, then view is updating but not the tab

yangwao · 2023-01-13T14:26:34Z

Hey, I'm happy we are closing on this, yet. I felt that, @prachi00, this issue was way too hard for you as the PR was opened over 2 weeks and accumulated a lot of comments and noise, which I don't prefer.

Next time you should estimate if you are able to finish issue in short time. Let's say 2-3 days and not block others (issues) if they can take over?
It's OK to make follow-up issue; commit what you have. Making it longer for the same payout won't help. I value if things are getting closed FAST. I'll probably write you on Discord to talk more about this :)

I really see it gets complex (34 files), yet it's better to leave room for others, we are playing on against time and distributing workload among other people :)

yangwao · 2023-01-13T14:27:06Z

pay 40 usd

yangwao · 2023-01-13T14:27:18Z

😍 Perfect, I’ve sent the payout
💵 $40 @ 27.74 USD/KSM ~ 1.442 $KSM
🧗 EzGc4s9PgCPx1YnF3fqzhLzVHpHMTL4LWPScwpDrR8JKgSU
🔗 0x661034be70b5488286137dcf16cfdd1f0f09bb77cc574d0f87ed850da55f9b03

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

prachi00 · 2023-01-13T17:37:56Z

Hey, I'm happy we are closing on this, yet. I felt that, @prachi00, this issue was way too hard for you as the PR was opened over 2 weeks and accumulated a lot of comments and noise, which I don't prefer.

Next time you should estimate if you are able to finish issue in short time. Let's say 2-3 days and not block others (issues) if they can take over? It's OK to make follow-up issue; commit what you have. Making it longer for the same payout won't help. I value if things are getting closed FAST. I'll probably write you on Discord to talk more about this :)

I really see it gets complex (34 files), yet it's better to leave room for others, we are playing on against time and distributing workload among other people :)

yeah I agree it did take time with reviews and stuff, as I had to migrate oruga tabs to a common component in library and use that, but then we decided to not use that and shift to buttons, so it did take time. I'll try to divide it into smaller tasks next time for sure.

prachi00 added 3 commits December 27, 2022 22:05

#4555 Explorer Tabs bar route update

43aa440

tabs ui

884dfbe

remove consoles

9e39ae8

prachi00 requested a review from a team as a code owner December 28, 2022 22:40

prachi00 requested review from roiLeo and removed request for a team December 28, 2022 22:40

prachi00 added 2 commits December 28, 2022 14:42

deepscan issue

6b27a2c

deepscan issue 2

7c198e6

prachi00 changed the title ~~Feat explore route~~ #4555 explore route changes Dec 28, 2022

yangwao added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jan 1, 2023

roiLeo suggested changes Jan 2, 2023

View reviewed changes

components/rmrk/ExploreLayout/ExploreLayout.vue Outdated Show resolved Hide resolved

pages/_prefix/explore/items.vue Outdated Show resolved Hide resolved

components/rmrk/ExploreLayout/ExploreLayout.vue Outdated Show resolved Hide resolved

Merge branch 'main' into feat-explore-route

84ccbe1

prachi00 and others added 2 commits January 3, 2023 13:30

Merge branch 'main' into feat-explore-route

09aa93d

neo tab

dbee66f

prachi00 commented Jan 4, 2023

View reviewed changes

components/rmrk/ExploreLayout/ExploreLayout.vue Outdated Show resolved Hide resolved

Merge branch 'main' into feat-explore-route

f64cc2a

conflicts

3f46b51

vikiival requested changes Jan 11, 2023

View reviewed changes

yangwao removed the S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved label Jan 11, 2023

exezbcz added the S-visual-ok-✅ label Jan 11, 2023

prachi00 and others added 4 commits January 11, 2023 21:59

redirect explore

a0cde9d

Merge branch 'main' into feat-explore-route

6265ec6

changes

d8e013c

vue composition

65fef60

roiLeo suggested changes Jan 12, 2023

View reviewed changes

prachi00 and others added 2 commits January 12, 2023 18:58

Merge branch 'main' into feat-explore-route

aeb76d2

removing negative margin

b7570bd

prachi00 requested review from vikiival and roiLeo January 13, 2023 04:01

vikiival approved these changes Jan 13, 2023

View reviewed changes

roiLeo approved these changes Jan 13, 2023

View reviewed changes

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

yangwao merged commit 7a4d4c3 into main Jan 13, 2023

yangwao deleted the feat-explore-route branch January 13, 2023 14:27

This was referenced Jan 13, 2023

Explorer Mobile Tabs #4683

Closed

Explorer Pagination and View update #4684

Closed

#4528 navbar changes mobile #4580

Merged

#4555 explore route changes #4577

#4555 explore route changes #4577

Conversation

prachi00 commented Dec 28, 2022 • edited

PR Type

Context

Before submitting pull request, please make sure:

Optional

Had issue bounty label?

Community participation

Screenshot 📸

netlify bot commented Dec 28, 2022 • edited

✅ Deploy Preview for koda-nuxt ready!

kodabot commented Dec 28, 2022

prachi00 commented Dec 28, 2022

yangwao commented Jan 1, 2023

roiLeo left a comment

Choose a reason for hiding this comment

yangwao commented Jan 3, 2023

prachi00 commented Jan 4, 2023

yangwao commented Jan 4, 2023

yangwao commented Jan 4, 2023 • edited

roiLeo commented Jan 4, 2023

yangwao commented Jan 4, 2023

roiLeo commented Jan 4, 2023

prachi00 commented Jan 5, 2023

prachi00 commented Jan 5, 2023

vikiival left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roiLeo left a comment

Choose a reason for hiding this comment

prachi00 commented Jan 12, 2023

preschian commented Jan 12, 2023 • edited

roiLeo commented Jan 12, 2023

preschian commented Jan 12, 2023

roiLeo commented Jan 12, 2023

codeclimate bot commented Jan 13, 2023

vikiival left a comment

Choose a reason for hiding this comment

roiLeo left a comment • edited

Choose a reason for hiding this comment

Note

yangwao commented Jan 13, 2023 • edited

yangwao commented Jan 13, 2023

yangwao commented Jan 13, 2023

prachi00 commented Jan 13, 2023

prachi00 commented Dec 28, 2022 •

edited

netlify bot commented Dec 28, 2022 •

edited

yangwao commented Jan 4, 2023 •

edited

preschian commented Jan 12, 2023 •

edited

roiLeo left a comment •

edited

yangwao commented Jan 13, 2023 •

edited