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

chore: stabilize side navigation bar during loading and switching testing types #26953

Merged
merged 6 commits into from
Jun 8, 2023
Merged

chore: stabilize side navigation bar during loading and switching testing types #26953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d0810f8
chore: minor improvements to UI when loading Cypress in open mode
astone123 Jun 6, 2023
34f9f7c
[run ci]
astone123 Jun 6, 2023
ef89955
fix tests
astone123 Jun 6, 2023
0edded7
feedback
astone123 Jun 7, 2023
a957a30
Merge branch 'develop' into loading-improvements
astone123 Jun 7, 2023
b1311ed
add condition back
astone123 Jun 8, 2023
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
8 changes: 7 additions & 1 deletion packages/app/src/layouts/default.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ const resetErrorAndLoadConfig = (id: string) => {
}
}

const renderSidebar = !isRunMode
const renderSidebar = computed(() => {
if (currentRoute.name === 'Specs' && query.data.value) {
astone123 marked this conversation as resolved.
Show resolved Hide resolved
return !isRunMode && query.data.value?.currentProject?.isLoadingConfigFile === false
}

return !isRunMode
})

</script>
1 change: 0 additions & 1 deletion packages/app/src/navigation/SidebarNavigation.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
</button>
<div class="flex flex-col flex-1 ">
<SidebarNavigationHeader
v-if="props.gql"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Always render the component even if the query isn't finished yet - handle the loading state inside of the component

:gql="props.gql"
:is-nav-bar-expanded="isNavBarExpanded"
/>
Expand Down
18 changes: 15 additions & 3 deletions packages/app/src/navigation/SidebarNavigationHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<Tooltip
v-if="testingType"
v-if="testingType && props.gql"
placement="right"
:disabled="isNavBarExpanded"
:distance="8"
Expand Down Expand Up @@ -46,6 +46,17 @@
</div>
</template>
</Tooltip>
<div
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Show a loading spinner if the query hasn't resolved yet

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird, I thought props.gql was always defined (even if it's an empty object). I guess it's undefined at first?

Copy link
Contributor

Choose a reason for hiding this comment

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

Edit: you changed the definition of the props to include ?.

v-else
class="border-b flex shrink-0 border-gray-900 h-[64px] pl-[20px] items-center"
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of these classes could have been pulled out to a div surrounding the whole component so they did not have to be duplicated with what is on line 11. Not going to hold up the PR for that

>
<IconLoading
name="loading"
stroke-color="indigo-700"
fill-color="indigo-300"
class="shrink-0 h-[24px] mr-[20px] w-[24px] animate-spin"
/>
</div>
</template>

<script lang="ts" setup>
Expand All @@ -56,6 +67,7 @@ import Tooltip from '@packages/frontend-shared/src/components/Tooltip.vue'
import SwitchTestingTypeModal from './SwitchTestingTypeModal.vue'
import IconE2E from '~icons/cy/testing-type-e2e-solid-simple'
import IconComponent from '~icons/cy/testing-type-component-solid_x24'
import { IconLoading } from '@cypress-design/vue-icon'
import { useI18n } from '@cy/i18n'
import { SidebarNavigationHeaderBranchChangeDocument } from '../generated/graphql-test'

Expand Down Expand Up @@ -87,7 +99,7 @@ useSubscription({ query: SidebarNavigationHeaderBranchChangeDocument })
const showModal = ref(false)

const props = defineProps<{
gql: SidebarNavigationHeaderFragment
gql?: SidebarNavigationHeaderFragment
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check, gql can actually be undefined at some point in the lifecycle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in this case it can be undefined. Ultimately it comes from SidebarNavigationContainer where we pass

:gql="query.data.value"

I think that's why it can be undefined

Screenshot 2023-06-07 at 3 19 31 PM

isNavBarExpanded: boolean
}>()

Expand All @@ -103,7 +115,7 @@ const TESTING_TYPE_MAP = {
} as const

const testingType = computed(() => {
return props.gql.currentProject?.currentTestingType ? TESTING_TYPE_MAP[props.gql.currentProject.currentTestingType] : null
return props.gql?.currentProject?.currentTestingType ? TESTING_TYPE_MAP[props.gql?.currentProject.currentTestingType] : null
})

</script>