-
-
Notifications
You must be signed in to change notification settings - Fork 347
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.
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
refactor: Navbar Vue composition api #5033
Conversation
SUCCESS @Jarsen136 PR for issue #5015 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime! |
✅ Deploy Preview for koda-nuxt ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They also occur on the beta environment. I have also seen some stuff colors change recently. related to #4855
👍 I will remove some unnecessary components import. Edit: I'm not sure if I miss something, but when I try to remove some component imports, some errors appear. |
Co-authored-by: roiLeo <medina.leo42@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usable
get isCreateVisible(): boolean { | ||
return createVisible(this.urlPrefix) | ||
} | ||
const isDarkMode = computed( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isnt there composable from vueuse? Cc @roiLeo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const colorMode = useColorMode()
$consola.log(colorMode.preference)
$colorMode
from useNuxtApp
working tambien
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
context: You will get system
instead of dark/light
value, when you visit the preview env the first time and have not set any color mode. So, I introduced this method to solve the incorrect color mode issue.
IMO, we should make this solution a global composable method and use it everywhere to replace useColorMode
. That could be a follow-up issue.
get currentCollection() { | ||
return this.$store.getters['history/getCurrentlyViewedCollection'] || {} | ||
} | ||
const logoSrc = computed(() => (isDarkMode.value ? KodaBetaDark : KodaBeta)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Time for logo component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are talking about the inline image component. I have introduced @nuxtjs/svg
to make it in the previous PR, but finally do not merge it.
https://github.com/kodadot/nft-gallery/pull/4780/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<KodadotLogo>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if we would like to use this kind of component, we need introduce a image loader, like @nuxtjs/svg
https://www.npmjs.com/package/@nuxtjs/svg
|
||
get showSearchOnNavbar(): boolean { | ||
return !this.isLandingPage || !this.showTopNavbar || this.isBurgerMenuOpened | ||
const onScroll = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also @preschian used some composable for bottom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean TBH 👀 Which part or composable api could I reuse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean TBH 👀
Was thinking if something like this would help https://vueuse.org/core/useScroll/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some trying, I found that useScroll
could not be used in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So let it roll
Co-authored-by: Viki Val <viktorko99@gmail.com>
Code Climate has analyzed commit 9def05f and detected 0 issues on this pull request. View more on Code Climate. |
✅ Fixed |
pay 40 usd |
😍 Perfect, I’ve sent the payout 🪅 Let’s grab another issue and get rewarded! |
pay 40 usd |
😍 Perfect, I’ve sent the payout 🪅 Let’s grab another issue and get rewarded! |
Thank you for your contribution to the KodaDot NFT gallery.
👇 _ Let's make a quick check before the contribution.
PR Type
Context
reduce LOC from 318 to 268
Before submitting pull request, please make sure:
Optional
Did your issue had any of the "$" label on it?
Community participation
Screenshot 📸