Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/web.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
- uses: pnpm/action-setup@v6
name: Install pnpm
with:
version: 11
version: 11.11.0

- name: Install dependencies 📦
run: pnpm install
Expand Down Expand Up @@ -66,7 +66,7 @@ jobs:
- uses: pnpm/action-setup@v6
name: Install pnpm
with:
version: 11
version: 11.11.0

- name: Install dependencies 📦
run: pnpm install
Expand Down
2 changes: 2 additions & 0 deletions web/app/components/MessageThreadHeader.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const phonesStore = usePhonesStore()
const threadsStore = useThreadsStore()
const appStore = useAppStore()
const notificationsStore = useNotificationsStore()
const redirectPreferenceStore = useRedirectPreferenceStore()

const selectedMenuItem = ref(-1)

Expand Down Expand Up @@ -71,6 +72,7 @@ async function logout() {
authStore.resetState()
phonesStore.resetState()
threadsStore.resetState()
redirectPreferenceStore.resetState()
notificationsStore.addNotification({
type: 'info',
message: 'You have successfully logged out',
Expand Down
56 changes: 56 additions & 0 deletions web/app/components/RedirectPromptPopover.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<script setup lang="ts">
import { mdiClose, mdiArrowRight } from '@mdi/js'

const route = useRoute()
const authStore = useAuthStore()
const redirectStore = useRedirectPreferenceStore()

const showPopover = computed(
() =>
route.name === 'index' &&
authStore.authUser !== null &&
!redirectStore.enabled &&
!redirectStore.dismissedThisSession,
)
</script>

<template>
<v-card
v-if="showPopover"
class="redirect-prompt pa-4"
elevation="8"
rounded="lg"
max-width="280"
>
<div class="d-flex align-center justify-space-between">
<span class="text-body-1">Skip this page next time?</span>
<v-btn
:icon="mdiClose"
variant="text"
size="small"
color="warning"
density="comfortable"
aria-label="Dismiss"
@click="redirectStore.dismiss()"
/>
</div>
<a
class="text-primary text-decoration-none hover:text-decoration-underline d-inline-flex align-center mt-1"
href="#"
@click.prevent="redirectStore.enable()"
>
Always open dashboard
<v-icon :icon="mdiArrowRight" size="small" class="ml-1" />
</a>
</v-card>
</template>

<style scoped>
.redirect-prompt {
position: absolute;
right: 0;
top: 100%;
margin-top: 8px;
z-index: 10;
}
</style>
21 changes: 13 additions & 8 deletions web/app/layouts/website.vue
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,21 @@ function goToPricing() {
Get Started
<span v-show="lgAndUp">&nbsp;For Free</span>
</v-btn>
<v-btn
<div
v-show="authStore.authUser !== null"
color="primary"
variant="flat"
:class="{ 'mt-5': mdAndUp, 'mt-1': !mdAndUp }"
:size="lgAndUp ? 'large' : 'default'"
:to="{ name: 'threads' }"
class="position-relative d-inline-block"
>
Dashboard
</v-btn>
<v-btn
color="primary"
variant="flat"
:class="{ 'mt-5': mdAndUp, 'mt-1': !mdAndUp }"
:size="lgAndUp ? 'large' : 'default'"
:to="{ name: 'threads' }"
>
Dashboard
</v-btn>
<RedirectPromptPopover />
</div>
</v-col>
</v-row>
</v-container>
Expand Down
9 changes: 9 additions & 0 deletions web/app/middleware/redirectToThreads.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default defineNuxtRouteMiddleware(() => {
try {
if (localStorage.getItem('httpsms_redirect_to_threads') === 'true') {
return navigateTo('/threads', { replace: true })
}
} catch (error) {
console.error(error)
}
})
Comment on lines +1 to +9

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The localStorage key is hardcoded here as a string literal, while the store defines it as a constant STORAGE_KEY. If the key ever needs to change, this file won't be caught by a simple search-and-replace on the constant, and the redirect middleware will silently stop working while the store continues writing the new key.

Suggested change
export default defineNuxtRouteMiddleware(() => {
try {
if (localStorage.getItem('httpsms_redirect_to_threads') === 'true') {
return navigateTo('/threads', { replace: true })
}
} catch (error) {
console.error(error)
}
})
import { STORAGE_KEY } from '~/stores/redirectPreference'
export default defineNuxtRouteMiddleware(() => {
try {
if (localStorage.getItem(STORAGE_KEY) === 'true') {
return navigateTo('/threads', { replace: true })
}
} catch (error) {
console.error(error)
}
})

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

1 change: 1 addition & 0 deletions web/app/pages/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {

definePageMeta({
layout: 'website',
middleware: ['redirect-to-threads'],
})

useSeoMeta({
Expand Down
2 changes: 2 additions & 0 deletions web/app/pages/settings/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const authStore = useAuthStore()
const phonesStore = usePhonesStore()
const billingStore = useBillingStore()
const notificationsStore = useNotificationsStore()
const redirectPreferenceStore = useRedirectPreferenceStore()

const firebaseUser = ref<FirebaseUser | null>(null)
const gravatarUrl = ref<string | null>(null)
Expand Down Expand Up @@ -773,6 +774,7 @@ async function deleteUserAccount() {
await signOut(auth)
authStore.resetState()
phonesStore.resetState()
redirectPreferenceStore.resetState()
notificationsStore.addNotification({
type: 'info',
message: 'You have successfully logged out',
Expand Down
47 changes: 47 additions & 0 deletions web/app/stores/redirectPreference.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { defineStore } from 'pinia'
import { ref } from 'vue'

const STORAGE_KEY = 'httpsms_redirect_to_threads'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The STORAGE_KEY constant is not exported, so the middleware cannot import it and must hardcode the string. Exporting it removes the duplication.

Suggested change
const STORAGE_KEY = 'httpsms_redirect_to_threads'
export const STORAGE_KEY = 'httpsms_redirect_to_threads'


function readFlag(): boolean {
try {
return localStorage.getItem(STORAGE_KEY) === 'true'
} catch (error) {
console.error(error)
return false
}
}

export const useRedirectPreferenceStore = defineStore(

Check warning on line 15 in web/app/stores/redirectPreference.ts

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

web/app/stores/redirectPreference.ts#L15

Unsafe assignment of an error typed value.

Check warning on line 15 in web/app/stores/redirectPreference.ts

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

web/app/stores/redirectPreference.ts#L15

Unsafe call of an `error` type typed value.
'redirectPreference',
() => {
const enabled = ref(readFlag())
const dismissedThisSession = ref(false)

function enable() {
enabled.value = true
try {
localStorage.setItem(STORAGE_KEY, 'true')
} catch (error) {
console.error(error)
}
navigateTo('/threads', { replace: true })
}
Comment on lines +21 to +29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Navigation proceeds even when persistence fails

enabled.value = true is set unconditionally, then localStorage.setItem is attempted. If the write throws (e.g. private-mode quota denial), the catch block is entered but navigateTo('/threads', ...) still runs afterward. The user is taken to /threads for this session, but on their next visit the middleware finds no key in localStorage and the landing page renders again — giving the impression that the "Always open dashboard" action had no lasting effect.


function dismiss() {
dismissedThisSession.value = true
}

function resetState() {
enabled.value = false
dismissedThisSession.value = false
try {
localStorage.removeItem(STORAGE_KEY)
} catch (error) {
console.error(error)
}
}

return { enabled, dismissedThisSession, enable, dismiss, resetState }
},
)
Loading