Skip to content
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.

Sign up for GitHub

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

Do not stack several automatic dialogs #1549

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Do not stack several automatic dialogs #1549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2855924
Store: App-interface: Add dialog queue
ArturoManzoli Jan 7, 2025
f6e9ea4
App: Startup dialogs: Implement changes to run under dialog queue
ArturoManzoli Jan 7, 2025
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
50 changes: 40 additions & 10 deletions src/App.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,12 @@
</v-main>
</v-app>
<About v-if="showAboutDialog" @update:show-about-dialog="showAboutDialog = $event" />
rafaellehmkuhl marked this conversation as resolved.
Show resolved Hide resolved
rafaellehmkuhl marked this conversation as resolved.
Show resolved Hide resolved
<Tutorial :show-tutorial="interfaceStore.isTutorialVisible" />
<VideoLibraryModal :open-modal="interfaceStore.isVideoLibraryVisible" />
<VehicleDiscoveryDialog v-model="showDiscoveryDialog" show-auto-search-option />
<UpdateNotification v-if="isElectron()" />

<!-- Startup dialogs queue -->
<div v-if="activeDialog">
<component :is="activeDialog.component" v-bind="activeDialog.props" @close="interfaceStore.openNextDialogOnQueue" />
</div>
</template>

<script setup lang="ts">
Expand Down Expand Up @@ -375,6 +377,8 @@ const isSlidingOut = ref(false)
const simplifiedMainMenu = ref(false)
const windowHeight = ref(window.innerHeight)

const activeDialog = computed(() => interfaceStore.activeDialog)

const configMenu = [
{
icon: 'mdi-view-dashboard-variant',
Expand Down Expand Up @@ -726,17 +730,43 @@ onBeforeUnmount(() => {
const currentTopBarHeightPixels = computed(() => `${widgetStore.currentTopBarHeightPixels}px`)
const currentBottomBarHeightPixels = computed(() => `${widgetStore.currentBottomBarHeightPixels}px`)

const showDiscoveryDialog = ref(false)
const preventAutoSearch = useStorage('cockpit-prevent-auto-vehicle-discovery-dialog', false)

onMounted(() => {
onMounted(async () => {
interfaceStore.enqueueDialog({
id: 'Tutorial',
component: Tutorial,
props: {},
})
await runElectronStartup()
interfaceStore.openNextDialogOnQueue()
})

const runElectronStartup = async (): Promise<void> => {
if (isElectron()) {
interfaceStore.enqueueDialog({
id: 'UpdateNotification',
component: UpdateNotification,
props: {},
})
// Wait 5 seconds to check if we're connected to a vehicle
setTimeout(() => {
if (vehicleStore.isVehicleOnline) return
interfaceStore.enqueueDialog({
id: 'VehicleDiscoveryDialog',
component: VehicleDiscoveryDialog,
props: {},
})
if (interfaceStore.dialogQueue[0].id === 'VehicleDiscoveryDialog') {
interfaceStore.openNextDialogOnQueue()
}
}, 5000)
}
rafaellehmkuhl marked this conversation as resolved.
Show resolved Hide resolved
if (!isElectron() || preventAutoSearch.value) return
}

// Wait 5 seconds to check if we're connected to a vehicle
setTimeout(() => {
if (vehicleStore.isVehicleOnline) return
showDiscoveryDialog.value = true
}, 5000)
onBeforeUnmount(() => {
interfaceStore.dialogQueue = []
})
</script>

Expand Down
14 changes: 13 additions & 1 deletion src/components/InteractionDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
<template>
<v-dialog v-model="internalShowDialog" :persistent="persistent" :width="maxWidth || 'auto'">
<v-dialog
v-model="internalShowDialog"
:persistent="persistent"
Comment on lines -2 to +4
Copy link
Member

Choose a reason for hiding this comment

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

In this case are we coupling the interaction dialog to the queue system? It seems strange, since we use it for a lot of popups that are open explicitly by the user (and not automatically), like the about page, menus, error dialogs, etc.

It seems to me they should be completely uncoupled, and the queue system should live outside it and only for automatically-issued dialogs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other dialogs will be unchanged and uncoupled from the queue. The ones that will be queued are only those that can potentially stack up, like the ones on the Cockpit startup.

It's also done like this on snackbar queue systems. Whenever a message can stackup, you queue it.

Copy link
Member

Choose a reason for hiding this comment

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

I mean that the queue system should be external to the InteractionDialog.vue component, otherwise we are putting queue logic on dialogs that should not be queue-aware.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In reality the InteractionDialog is uncoupled from the queue system.

The queue uses a logic based on dependency injection. If you check on App.vue, the components are loaded on startup and queued independently and the enqueueDialog function takes the component, its props and an id as parameter.

interfaceStore.enqueueDialog({ id: 'VehicleDiscoveryDialog', component: VehicleDiscoveryDialog, props: {}, })

For example, the system update dialog will only be queued if the function that checks for new versions triggers the dialog and will inject the component on the function to do so.
Actually it doesn't have to be a dialog. Could be virtually any other component we have.

Copy link
Member

@rafaellehmkuhl rafaellehmkuhl Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What I'm saying is that the interaction dialog should not be the one deciding on what happens with the log queue (interfaceStore.openNextDialogOnQueue()), but instead there should be an observer outside the interaction dialog checking if there's any interaction dialog open, and if not, open the next dialog in the queue.

The interaction dialog should be what is is, a dialog. It does not need to be aware of the queue. It's an unnecessary architecture coupling.

It's the same problem we had before, that you're fixing in this PR, about the interaction dialog being aware of the Tutorial functionality (it shouldn't).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the InteractionDialog isn't aware of the queue. The only modification I made to it was the closing logic so it can handle the queue

Copy link
Member

Choose a reason for hiding this comment

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

Discussed the details in a call. Main topic was to uncouple to facilitate maintenance, shrink the codebase and allow easy future changes (for new auto-instantiated dialogs).

:width="maxWidth || 'auto'"
@click:outside="handleClose"
@close="handleClose"
>
<v-card
:width="maxWidth || 'auto'"
class="main-dialog px-2 rounded-lg"
Expand Down Expand Up @@ -200,6 +206,12 @@ const iconColor = computed(() => {

const hasActionsSlot = computed(() => !!slots.actions)

const handleClose = (): void => {
if (interfaceStore.dialogQueue.length > 0) {
interfaceStore.openNextDialogOnQueue()
}
}

watch(
() => props.showDialog,
(newVal) => {
Expand Down
19 changes: 4 additions & 15 deletions src/components/Tutorial.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,20 +91,11 @@ const { showSnackbar } = useSnackbar()
const interfaceStore = useAppInterfaceStore()
const vehicleStore = useMainVehicleStore()

const props = defineProps<{
/**
*
*/
showTutorial?: boolean
}>()

const emits = defineEmits(['update:showTutorial'])

const showTutorial = ref(props.showTutorial || false)
const userHasSeenTutorial = useBlueOsStorage('cockpit-has-seen-tutorial', false)
const showTutorial = ref((interfaceStore.activeDialog?.id === 'Tutorial' && !userHasSeenTutorial.value) || false)
const currentTutorialStep = ref(1)
const isVehicleConnectedVisible = ref(false)
const tallContent = ref(false)
const userHasSeenTutorial = useBlueOsStorage('cockpit-has-seen-tutorial', false)

const steps = [
{
Expand Down Expand Up @@ -345,7 +336,7 @@ const handleStepChangeDown = (newStep: number): void => {
const dontShowTutorialAgain = (): void => {
userHasSeenTutorial.value = true
showTutorial.value = false
emits('update:showTutorial', false)
interfaceStore.openNextDialogOnQueue()
showSnackbar({
message: 'This guide can be reopened via the Settings > General menu',
variant: 'info',
Expand All @@ -356,8 +347,6 @@ const dontShowTutorialAgain = (): void => {

const alwaysShowTutorialOnStartup = (): void => {
userHasSeenTutorial.value = false
showTutorial.value = true
emits('update:showTutorial', true)
}

const nextTutorialStep = (): void => {
Expand All @@ -379,7 +368,7 @@ const closeTutorial = (): void => {
showTutorial.value = false
interfaceStore.componentToHighlight = 'none'
currentTutorialStep.value = 1
emits('update:showTutorial', false)
interfaceStore.openNextDialogOnQueue()
}

const setVehicleConnectedVisible = (): void => {
Expand Down
35 changes: 25 additions & 10 deletions src/components/UpdateNotification.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
</template>
</v-progress-linear>
</template>
<template #actions> <v-btn variant="text" size="small" @click="handleClose">Close</v-btn> </template>
</InteractionDialog>
</template>

Expand All @@ -37,6 +38,9 @@ import { onBeforeMount, ref } from 'vue'
import InteractionDialog, { type Action } from '@/components/InteractionDialog.vue'
import { app_version } from '@/libs/cosmos'
import { isElectron } from '@/libs/utils'
import { useAppInterfaceStore } from '@/stores/appInterface'

const interfaceStore = useAppInterfaceStore()

const showUpdateDialog = ref(false)
const dialogTitle = ref('')
Expand All @@ -52,6 +56,11 @@ const updateInfo = ref({
})
const ignoredUpdateVersions = useStorage<string[]>('cockpit-ignored-update-versions', [])

const handleClose = (): void => {
interfaceStore.openNextDialogOnQueue()
showUpdateDialog.value = false
}

const formatDate = (date: string): string => {
return new Date(date).toLocaleDateString('en-US', { year: 'numeric', month: 'long', day: 'numeric' })
}
Expand All @@ -75,7 +84,9 @@ onBeforeMount(() => {
dialogVariant.value = 'info'
dialogActions.value = []
showProgress.value = false
showUpdateDialog.value = true
if (interfaceStore.activeDialog?.id === 'UpdateNotification' || interfaceStore.activeDialog === undefined) {
showUpdateDialog.value = true
}
})

window.electronAPI.onUpdateNotAvailable(() => {
Expand All @@ -87,7 +98,7 @@ onBeforeMount(() => {
{
text: 'OK',
action: () => {
showUpdateDialog.value = false
interfaceStore.openNextDialogOnQueue()
},
},
]
Expand All @@ -107,7 +118,7 @@ onBeforeMount(() => {
console.log(`User chose to ignore version ${updateInfo.value.version}`)
ignoredUpdateVersions.value.push(updateInfo.value.version)
window.electronAPI!.cancelUpdate()
showUpdateDialog.value = false
interfaceStore.openNextDialogOnQueue()
},
},
{
Expand All @@ -121,7 +132,7 @@ onBeforeMount(() => {
action: () => {
console.log('User chose to cancel the update for the Electron app.')
window.electronAPI!.cancelUpdate()
showUpdateDialog.value = false
interfaceStore.openNextDialogOnQueue()
dialogMessage.value = 'Downloading update...'
},
},
Expand All @@ -132,19 +143,21 @@ onBeforeMount(() => {
text: 'Not Now',
action: () => {
window.electronAPI!.cancelUpdate()
showUpdateDialog.value = false
interfaceStore.openNextDialogOnQueue()
},
},
]

// Check if this version is in the ignored list
if (ignoredUpdateVersions.value.includes(info.version)) {
console.log(`Skipping ignored version ${info.version}.`)
showUpdateDialog.value = false
interfaceStore.openNextDialogOnQueue()
return
}

showUpdateDialog.value = true
if (interfaceStore.activeDialog?.id === 'UpdateNotification' || interfaceStore.activeDialog === undefined) {
showUpdateDialog.value = true
}
})

window.electronAPI.onDownloadProgress((progressInfo) => {
Expand All @@ -164,18 +177,20 @@ onBeforeMount(() => {
action: () => {
console.log('User chose to install the update for the Electron app now.')
window.electronAPI!.installUpdate()
showUpdateDialog.value = false
interfaceStore.openNextDialogOnQueue()
},
},
{
text: 'Later',
action: () => {
console.log('User chose to install the update for the Electron app later.')
showUpdateDialog.value = false
interfaceStore.openNextDialogOnQueue()
},
},
]
showUpdateDialog.value = true
if (interfaceStore.activeDialog?.id === 'UpdateNotification' || interfaceStore.activeDialog === undefined) {
showUpdateDialog.value = true
}
})
})
</script>
14 changes: 11 additions & 3 deletions src/components/VehicleDiscoveryDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,18 @@

<script setup lang="ts">
import { useStorage } from '@vueuse/core'
import { ref, watch } from 'vue'
import { computed, ref, watch } from 'vue'

import { useSnackbar } from '@/composables/snackbar'
import vehicleDiscover, { NetworkVehicle } from '@/libs/electron/vehicle-discovery'
import { reloadCockpit } from '@/libs/utils'
import { useAppInterfaceStore } from '@/stores/appInterface'
import { useMainVehicleStore } from '@/stores/mainVehicle'

import InteractionDialog, { Action } from './InteractionDialog.vue'

const interfaceStore = useAppInterfaceStore()

const props = defineProps<{
/**
*
Expand All @@ -75,7 +78,8 @@ const { showSnackbar } = useSnackbar()
const mainVehicleStore = useMainVehicleStore()
const discoveryService = vehicleDiscover

const isOpen = ref(props.modelValue)
const isDialogOpen = computed(() => interfaceStore.activeDialog?.id === 'VehicleDiscoveryDialog' || props.modelValue)
const isOpen = ref(isDialogOpen.value || false)
const searching = ref(false)
const searched = ref(false)
const vehicles = ref<NetworkVehicle[]>([])
Expand All @@ -85,6 +89,7 @@ const originalActions = [
{
text: 'Close',
action: () => {
interfaceStore.openNextDialogOnQueue()
isOpen.value = false
},
},
Expand All @@ -105,7 +110,6 @@ watch(
isOpen.value = value
}
)

watch(isOpen, (value) => {
emit('update:modelValue', value)
})
Expand All @@ -114,6 +118,8 @@ const searchVehicles = async (): Promise<void> => {
searching.value = true
disableButtons()
vehicles.value = await discoveryService.findVehicles()
interfaceStore.openNextDialogOnQueue()
isOpen.value = false
searching.value = false
enableButtons()
searched.value = true
Expand All @@ -122,6 +128,7 @@ const searchVehicles = async (): Promise<void> => {
const selectVehicle = async (address: string): Promise<void> => {
mainVehicleStore.globalAddress = address
isOpen.value = false
interfaceStore.openNextDialogOnQueue()
await reloadCockpit()
showSnackbar({ message: 'Vehicle address updated', variant: 'success', duration: 5000 })
}
Expand All @@ -131,6 +138,7 @@ const preventFutureAutoSearchs = (): void => {
disableButtons()
setTimeout(() => {
isOpen.value = false
interfaceStore.openNextDialogOnQueue()
}, 5000)
}

Expand Down
11 changes: 11 additions & 0 deletions src/stores/appInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { watch } from 'vue'

import { defaultDisplayUnitPreferences } from '@/assets/defaults'
import { useBlueOsStorage } from '@/composables/settingsSyncer'
import { DialogOnQueue } from '@/types/ui'

const { width: windowWidth, height: windowHeight } = useWindowSize()

Expand All @@ -28,6 +29,8 @@ export const useAppInterfaceStore = defineStore('responsive', {
isGlassModalAlwaysOnTop: false,
isTutorialVisible: false,
configPanelVisible: false,
dialogQueue: [] as DialogOnQueue[],
activeDialog: undefined as DialogOnQueue | undefined,
}),
actions: {
updateWidth() {
Expand All @@ -41,6 +44,14 @@ export const useAppInterfaceStore = defineStore('responsive', {
.padStart(2, '0')
this.UIGlassEffect.bgColor = `#${hex}${alphaHex}`
},
enqueueDialog(dialog: DialogOnQueue) {
this.dialogQueue.push(dialog)
},
openNextDialogOnQueue() {
if (this.dialogQueue.length > 0) {
this.activeDialog = this.dialogQueue.shift()
}
},
},
getters: {
isXs: (state) => state.width < 720, // Extra small devices (5-6" mobile screens in landscape)
Expand Down
14 changes: 14 additions & 0 deletions src/types/ui.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export type DialogOnQueue = {
/**
* Dialog Id
*/
id: string
/**
* Component to be rendered
*/
component: any
/**
* Props to be passed to the component
*/
props?: Record<string, any>
}
Loading