-
-
Notifications
You must be signed in to change notification settings - Fork 565
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
fix: change page reload and account switch logic #2975
Conversation
✅ Deploy Preview for elk-docs canceled.
|
✅ Deploy Preview for elk-zone ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Looks like we have some problem with sign-in logic (I added some await, FF and Chrome ara not able to load home page) |
I need to review the |
Cannot sign out in PR preview. |
components/nav/NavSide.vue
Outdated
return '/notifications' | ||
} | ||
|
||
return `/notifications/${lastAccessedNotificationRoute}` |
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.
shouldn't eslint fail here (computed without .value
)?
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.
RemovableRef
is a type from VueUse using type instead interface (type RemovableRef<T> = Omit<Ref, 'value'> & {...}
), maybe some check not being resolved: https://github.com/vuejs/eslint-plugin-vue/blob/2dc606ca589ff5cdcd197c3c64ace7e575a6347d/lib/utils/index.js#L1130
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.
Tested on Firefox 130.0.1 (64-bit) on Linux.
Although I didn't fully trace the code change, I confirmed that
- the reported reloading bug was fixed
- list and hashtag pages are working as previously
- last selected notification and explorer tabs are remembered correctly
- there's reloading happening when switching accounts but I think that should not affect usability so significantly and acceptable for now
Thanks for the quick resolution folks! Really appreciate it! |
This PR includes:
visibilty
listeners.fix masto plugin, rn calling masto in the servermasto.ts
plugin and moved the logic to0.setup-users.ts
plugin: client logic will activate the watcher once user signed (✔️ we should usefinally
insteadthen
: DONE)NOTE: almost impossible to test 2 previous fixes from main, any page refresh and the current token broken (masto error)
Error building elk.zone (nitropack patched locally)
EDIT: no idea what's doing Google when releasing v129
closes #2972