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
7a381f4
Composables: Add dialog queue composable
ArturoManzoli Feb 17, 2025
4b74a31
App: Startup dialogs: Implement changes to run under dialog queue
ArturoManzoli Feb 17, 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
34 changes: 21 additions & 13 deletions src/App.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,11 @@
</v-main>
</v-app>
<About v-if="showAboutDialog" @update:show-about-dialog="showAboutDialog = $event" />
<Tutorial v-if="interfaceStore.isTutorialVisible" />
<VideoLibraryModal v-if="interfaceStore.isVideoLibraryVisible" />
<VehicleDiscoveryDialog v-model="showDiscoveryDialog" show-auto-search-option />
<UpdateNotification v-if="isElectron()" />
<Tutorial :model-value="interfaceStore.isTutorialVisible" />
<div v-if="WrappedDialog">
<component :is="WrappedDialog" />
</div>
Comment on lines -313 to +317
Copy link
Member

Choose a reason for hiding this comment

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

To better understand: shouldn't the Tutorial be used in the queue system, since it's opened automatically as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not for the queue. this for the manual opening on the General settings. There is also a similar implementation for the vehicle discovery dialog.
The other way to manually open those, is to create a single item queue for them when manually called.

Copy link
Member

@rafaellehmkuhl rafaellehmkuhl Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

So if we open those manually and they are triggered to be open automatically, there will be two instances of them in the screen, not?

Copy link
Member

Choose a reason for hiding this comment

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

@ArturoManzoli

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe yes, but I see them far from each other in the sense of UI and how the user will use either the auto pop up and the manual instance of it.

Would be a big problem if the user manage to intentionally open two instances of those dialogs?

If so, is better to also enqueue the manual opening.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see a problem with this as long as they are not the same dialog.

If the user has a tutorial that was automatically open, it's fine if they open a vehicle discovery one manually, and this manual one gets on top of the other, but if the user has a vehicle discovery manually opened, an automatic one should not be opened over it.

Does it make sense?

<SnackbarContainer />
</template>

Expand All @@ -336,6 +337,7 @@ import Tutorial from '@/components/Tutorial.vue'
import UpdateNotification from '@/components/UpdateNotification.vue'
import VehicleDiscoveryDialog from '@/components/VehicleDiscoveryDialog.vue'
import VideoLibraryModal from '@/components/VideoLibraryModal.vue'
import { useDialogQueue } from '@/composables/dialogQueue'
import { useInteractionDialog } from '@/composables/interactionDialog'
import {
availableCockpitActions,
Expand Down Expand Up @@ -366,6 +368,8 @@ import ConfigurationUIView from './views/ConfigurationUIView.vue'
import ConfigurationVideoView from './views/ConfigurationVideoView.vue'
import ToolsDataLakeView from './views/ToolsDataLakeView.vue'
import ToolsMAVLinkView from './views/ToolsMAVLinkView.vue'

const { enqueueDialog, WrappedDialog } = useDialogQueue()
const { showDialog, closeDialog } = useInteractionDialog()
const { openSnackbar } = useSnackbar()

Expand Down Expand Up @@ -760,23 +764,27 @@ 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(() => {
if (isElectron() && !preventAutoSearch.value) {
if (!interfaceStore.userHasSeenTutorial) {
enqueueDialog({
component: Tutorial,
})
}
if (isElectron()) {
enqueueDialog({
component: UpdateNotification,
})
// Wait 5 seconds to check if we're connected to a vehicle
setTimeout(() => {
if (vehicleStore.isVehicleOnline) return
showDiscoveryDialog.value = true
if (vehicleStore.isVehicleOnline || preventAutoSearch.value) return
enqueueDialog({
component: VehicleDiscoveryDialog,
props: { showAutoSearchOption: true },
})
}, 5000)
}

if (!interfaceStore.userHasSeenTutorial) {
setTimeout(() => {
interfaceStore.isTutorialVisible = true
}, 6000)
}
})
</script>

Expand Down
28 changes: 14 additions & 14 deletions src/components/Tutorial.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,18 @@ const { openSnackbar } = useSnackbar()
const interfaceStore = useAppInterfaceStore()
const vehicleStore = useMainVehicleStore()

const showTutorial = ref(true)
const props = defineProps<{
/**
* Parent-controlled trigger for showing the dialog.
*/
modelValue: boolean
}>()

const emit = defineEmits<{
(e: 'update:modelValue', value: boolean): void
}>()

const showTutorial = ref(false || props.modelValue)
const currentTutorialStep = useStorage('cockpit-last-tutorial-step', 1)
const isVehicleConnectedVisible = ref(false)
const tallContent = ref(false)
Expand Down Expand Up @@ -208,31 +219,27 @@ const handleStepChange = (newStep: number): void => {
interfaceStore.componentToHighlight = 'menu-trigger'
interfaceStore.currentSubMenuComponentName = null
interfaceStore.currentSubMenuName = null
interfaceStore.userHasSeenTutorial = false
break
case 3:
interfaceStore.isMainMenuVisible = true
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.componentToHighlight = 'settings-menu-item'
interfaceStore.currentSubMenuComponentName = null
interfaceStore.userHasSeenTutorial = false
break
case 4:
interfaceStore.isMainMenuVisible = true
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsGeneral
tallContent.value = true
interfaceStore.userHasSeenTutorial = false
interfaceStore.componentToHighlight = 'General'
break
case 5:
interfaceStore.isMainMenuVisible = true
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsGeneral
interfaceStore.userHasSeenTutorial = false
tallContent.value = false
interfaceStore.componentToHighlight = 'vehicle-address'
setVehicleConnectedVisible()
Expand All @@ -243,15 +250,13 @@ const handleStepChange = (newStep: number): void => {
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsInterface
tallContent.value = false
interfaceStore.userHasSeenTutorial = false
interfaceStore.componentToHighlight = 'Interface'
break
case 7:
interfaceStore.isMainMenuVisible = true
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsJoystick
interfaceStore.userHasSeenTutorial = false
tallContent.value = true
interfaceStore.componentToHighlight = 'Joystick'
break
Expand All @@ -260,7 +265,6 @@ const handleStepChange = (newStep: number): void => {
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsVideo
interfaceStore.userHasSeenTutorial = false
tallContent.value = true
interfaceStore.componentToHighlight = 'Video'
break
Expand All @@ -269,7 +273,6 @@ const handleStepChange = (newStep: number): void => {
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsTelemetry
interfaceStore.userHasSeenTutorial = false
tallContent.value = false
interfaceStore.isGlassModalAlwaysOnTop = true
interfaceStore.componentToHighlight = 'Telemetry'
Expand All @@ -279,7 +282,6 @@ const handleStepChange = (newStep: number): void => {
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsAlerts
interfaceStore.userHasSeenTutorial = false
tallContent.value = false
interfaceStore.isGlassModalAlwaysOnTop = false
interfaceStore.componentToHighlight = 'Alerts'
Expand All @@ -289,7 +291,6 @@ const handleStepChange = (newStep: number): void => {
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsDev
interfaceStore.userHasSeenTutorial = false
tallContent.value = true
interfaceStore.isGlassModalAlwaysOnTop = false
interfaceStore.componentToHighlight = 'Dev'
Expand All @@ -299,7 +300,6 @@ const handleStepChange = (newStep: number): void => {
interfaceStore.mainMenuCurrentStep = 2
interfaceStore.currentSubMenuName = SubMenuName.settings
interfaceStore.currentSubMenuComponentName = SubMenuComponentName.SettingsMission
interfaceStore.userHasSeenTutorial = false
tallContent.value = false
interfaceStore.isGlassModalAlwaysOnTop = false
interfaceStore.componentToHighlight = 'Mission'
Expand All @@ -316,6 +316,7 @@ const handleStepChange = (newStep: number): void => {
const dontShowTutorialAgain = (): void => {
interfaceStore.userHasSeenTutorial = true
showTutorial.value = false
emit('update:modelValue', false)
currentTutorialStep.value = 1
openSnackbar({
message: 'This guide can be reopened via the Settings > General menu',
Expand All @@ -332,7 +333,6 @@ const alwaysShowTutorialOnStartup = (): void => {

const nextTutorialStep = (): void => {
if (currentTutorialStep.value === steps.length) {
dontShowTutorialAgain()
closeTutorial()
return
}
Expand All @@ -348,8 +348,8 @@ const backTutorialStep = (): void => {
const closeTutorial = (): void => {
showTutorial.value = false
interfaceStore.componentToHighlight = 'none'
interfaceStore.userHasSeenTutorial = true
interfaceStore.isTutorialVisible = false
emit('update:modelValue', false)
}

const setVehicleConnectedVisible = (): void => {
Expand Down
58 changes: 47 additions & 11 deletions src/components/UpdateNotification.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<InteractionDialog
v-model="showUpdateDialog"
:show-dialog="showUpdateDialog"
:title="dialogTitle"
:message="dialogMessage"
:variant="dialogVariant"
Expand All @@ -27,17 +27,34 @@
</template>
</v-progress-linear>
</template>
<template #actions>
<v-btn variant="text" size="small" @click="handleCloseDialog">Close</v-btn>
</template>
</InteractionDialog>
</template>

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

import InteractionDialog, { type Action } from '@/components/InteractionDialog.vue'
import { openSnackbar } from '@/composables/snackbar'
import { app_version } from '@/libs/cosmos'
import { isElectron } from '@/libs/utils'

defineProps<{
/**
* Parent-controlled trigger for showing the dialog.
*/
// eslint-disable-next-line vue/no-unused-properties
modelValue: boolean
}>()

const emit = defineEmits<{
(e: 'update:modelValue', value: boolean): void
}>()

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

// Wait for 2 seconds for updates. If no updates are available, show a message and emit close.
onMounted(() => {
setTimeout(() => {
if (updateAvailable.value) {
showUpdateDialog.value = true
} else {
openSnackbar({ message: 'No updates available.', variant: 'success' })
Copy link
Member

Choose a reason for hiding this comment

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

This notification is wrong. If there's an update available but the modelValue props is false, it will enter here.

emit('update:modelValue', false)
}
}, 2000)
})

const handleCloseDialog = (): void => {
emit('update:modelValue', false)
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 +109,7 @@ onBeforeMount(() => {
dialogVariant.value = 'info'
dialogActions.value = []
showProgress.value = false
showUpdateDialog.value = true
updateAvailable.value = true
})

window.electronAPI.onUpdateNotAvailable(() => {
Expand All @@ -87,11 +121,12 @@ onBeforeMount(() => {
{
text: 'OK',
action: () => {
showUpdateDialog.value = false
handleCloseDialog()
},
},
]
showProgress.value = false
updateAvailable.value = false
})

window.electronAPI.onUpdateAvailable((info) => {
Expand All @@ -107,7 +142,7 @@ onBeforeMount(() => {
console.log(`User chose to ignore version ${updateInfo.value.version}`)
ignoredUpdateVersions.value.push(updateInfo.value.version)
window.electronAPI!.cancelUpdate()
showUpdateDialog.value = false
handleCloseDialog()
},
},
{
Expand All @@ -121,7 +156,7 @@ onBeforeMount(() => {
action: () => {
console.log('User chose to cancel the update for the Electron app.')
window.electronAPI!.cancelUpdate()
showUpdateDialog.value = false
handleCloseDialog()
dialogMessage.value = 'Downloading update...'
},
},
Expand All @@ -132,19 +167,20 @@ onBeforeMount(() => {
text: 'Not Now',
action: () => {
window.electronAPI!.cancelUpdate()
showUpdateDialog.value = false
handleCloseDialog()
},
},
]

// 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

handleCloseDialog()
return
}

showUpdateDialog.value = true
updateAvailable.value = true
})

window.electronAPI.onDownloadProgress((progressInfo) => {
Expand All @@ -171,11 +207,11 @@ onBeforeMount(() => {
text: 'Later',
action: () => {
console.log('User chose to install the update for the Electron app later.')
showUpdateDialog.value = false
handleCloseDialog()
},
},
]
showUpdateDialog.value = true
updateAvailable.value = true
})
})
</script>
Loading