Skip to content

fix: app layout + specs list review #18862

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

Merged
merged 8 commits into from
Nov 15, 2021
Merged

fix: app layout + specs list review #18862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
186b936
fix default layout
elevatebart Nov 9, 2021
22ff013
Know where to find the specs
elevatebart Nov 9, 2021
e82c0b6
make specs line oveflow
elevatebart Nov 9, 2021
9338259
Merge branch 'unified-desktop-gui' into refactor/app-layout
Nov 9, 2021
9f485c5
reset base.scss
elevatebart Nov 9, 2021
7a1f009
and fix a dancing hover on buttons
elevatebart Nov 9, 2021
b42fa39
fix typescript check
elevatebart Nov 10, 2021
6ea06ba
merge in origin
lmiller1990 Nov 15, 2021
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
73 changes: 36 additions & 37 deletions packages/app/src/layouts/default.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,42 @@
<template>
<div class="h-screen overflow-hidden flex flex-row">
<main class="h-screen min-w-0 flex-1 lg:flex">
<section
aria-labelledby="primary-heading"
class="min-w-0 flex-1 h-full flex flex-col overflow-hidden lg:order-last"
>
<HeaderBar
v-if="showHeader"
:show-browsers="true"
:page-name="pageName"
/>
<router-view v-slot="{ Component, route }">
<h1
id="primary-heading"
class="sr-only"
>
{{ route.name }}
</h1>
<transition
name="fade"
mode="out-in"
>
<!-- <keep-alive> -->
<component :is="Component" />
<!-- </keep-alive> -->
</transition>
</router-view>
</section>
</main>
<div
class="h-screen order-first transition-all"
:class="mainStore.navBarExpanded ? 'w-248px' : 'w-64px'"
<div class="h-screen layout-grid">
<SidebarNavigation class="row-span-full" />

<HeaderBar
v-if="showHeader"
:show-browsers="true"
:page-name="pageName"
/>

<main
class="overflow-y-auto"
aria-labelledby="primary-heading"
>
<SidebarNavigation />
</div>
<div id="tooltip-target" />
<router-view v-slot="{ Component, route }">
<h1
id="primary-heading"
class="sr-only"
>
{{ route.name }}
</h1>
<transition
name="fade"
mode="out-in"
>
<component :is="Component" />
</transition>
</router-view>
</main>
</div>
<div id="tooltip-target" />
</template>

<script lang="ts" setup>
import SidebarNavigation from '../navigation/SidebarNavigation.vue'
import HeaderBar from '@cy/gql-components/HeaderBar.vue'
import { useRoute } from 'vue-router'
import { computed } from 'vue'
import { useMainStore } from '../store'

const mainStore = useMainStore()
const currentRoute = useRoute()

const showHeader = computed(() => {
Expand All @@ -56,3 +47,11 @@ const pageName = computed((): string | undefined => {
return currentRoute.meta?.title as string
})
</script>

<style lang="scss" scoped>
.layout-grid {
display: grid;
grid-template-columns: auto 1fr;
grid-template-rows: 64px 1fr;
}
</style>
12 changes: 7 additions & 5 deletions packages/app/src/navigation/SidebarNavigation.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
<template>
<HideDuringScreenshot class="flex relative flex-col flex-1 h-full bg-gray-1000">
<HideDuringScreenshot
:aria-expanded="mainStore.navBarExpanded"
class="relative flex flex-col bg-gray-1000 transition-all duration-300"
:class="mainStore.navBarExpanded ? 'w-248px' : 'w-64px'"
>
<div
class="absolute cursor-pointer w-16px bottom-0 top-0 left-full group"
:aria-expanded="mainStore.navBarExpanded"
@click="mainStore.toggleNavBar"
>
<div class="w-16px origin-left transform scale-x-0 group-hover:scale-x-100 h-full transition-transform duration-300 flex items-center">
Expand All @@ -15,7 +18,7 @@
</div>
<div class="flex flex-col flex-1 overflow-y-auto ">
<SidebarTooltip
class="flex items-center h-64px"
class="flex items-center h-64px flex-shrink-0 border-b border-gray-900"
:disabled="mainStore.navBarExpanded"
:popper-top-offset="4"
popper-class="h-56px"
Expand Down Expand Up @@ -45,7 +48,6 @@
</template>
</SidebarTooltip>

<hr class="border-gray-900">
<nav
class="flex-1 space-y-1 bg-gray-1000"
aria-label="Sidebar"
Expand Down Expand Up @@ -117,7 +119,7 @@ import { useI18n } from '@cy/i18n'
const { t } = useI18n()

const navigation = [
{ name: 'Home', icon: CodeIcon, href: '/' },
{ name: 'Specs', icon: CodeIcon, href: '/' },
{ name: 'Runs', icon: RunsIcon, href: '/runs' },
{ name: 'Settings', icon: SettingsIcon, href: '/settings' },
]
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/pages/Runs.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<div class="relative p-24px h-full overflow-y-scroll">
<div class="relative p-24px h-full">
<TransitionQuickFade>
<RunsSkeleton v-if="query.fetching.value || !query.data.value" />
<RunsPage
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/settings/SettingsContainer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<div
class="p-24px h-full overflow-auto"
class="p-24px h-full"
data-cy="settings"
>
<SettingsCard
Expand Down
12 changes: 9 additions & 3 deletions packages/app/src/specs/SpecsListRow.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
:is="icon.component"
:class="icon.classes"
/>
<div>
<div
class="overflow-hidden truncate text-gray-400 group-hocus:text-indigo-500"
:title="`${spec.fileName}${spec.specFileExtension}`"
>
<span class="font-medium text-gray-700 group-hocus:text-indigo-500">{{ spec.fileName }}</span>
<span class="font-light text-gray-400 group-hocus:text-indigo-500">{{ spec.specFileExtension }}</span>
<span class="font-light group-hocus:text-indigo-500">{{ spec.specFileExtension }}</span>
</div>
</div>
<div class="grid git-info-row grid-cols-[16px,auto] items-center gap-9px">
Expand All @@ -27,7 +30,10 @@
class="min-w-16px max-w-16px min-h-16px icon-dark-gray-300 group-hocus:icon-dark-indigo-500 max-h-16px"
/>
</span>
<div :class="classes?.gitText">
<div
class="overflow-hidden truncate"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not so sure about truncating. If we don't want to make the rows taller & wrap the text, maybe we could allow horizontal scroll. Mainly I'm thinking if the data is useful at full width, it's still useful if somebody is choosing to make the window smaller - or triggers the truncate through browser zoom controls.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about :title="specName"? You could just hover it with your mouse.

Copy link
Contributor

@lmiller1990 lmiller1990 Nov 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I resolved the conflicts with the main branch and added :title along the way, so if the text is truncated, there's at least some way to see the full title (by hovering). Happy to discuss the UI further - since the correct behavior isn't obvious from the mocks, I'm just going to merge this one up since I don't see any obvious problems.

Let's discuss how this should behave, once we have a consensus on the correct behavior, we can make another PR to amend it.

:class="classes?.gitText"
>
{{ gitInfoText }}
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/store/main-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const useMainStore = defineStore({
id: 'main',
state: (): MainStoreState => {
return {
navBarExpanded: false,
navBarExpanded: true,
}
},
actions: {
Expand Down
2 changes: 1 addition & 1 deletion packages/frontend-shared/.windicss/shortcuts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* outline with that, do border-transparent for the non-hocus state.
*/

const focusDefault = 'outline-none focus:border focus:border-indigo-300 focus:ring-2 focus:ring-indigo-100 focus:outline-transparent transition duration-150 disabled:hover:ring-0 disabled:hover:border-0'
const focusDefault = 'outline-none focus:border focus:border-indigo-300 focus:ring-2 focus:ring-indigo-100 focus:outline-transparent transition duration-150 disabled:hover:ring-0 disabled:hover:border-transparent'
Copy link
Contributor

Choose a reason for hiding this comment

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

nice fix!


// Usually what you want
const hocusDefault = focusDefault.replace(/focus:/g, 'hocus:')
Expand Down
6 changes: 6 additions & 0 deletions packages/frontend-shared/src/components/Button.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,10 @@ describe('<Button />', { viewportWidth: 300, viewportHeight: 300 }, () => {

cy.contains('a', 'test').should('not.have.attr', 'data-cy')
})

it('shows disabled properly', () => {
cy.mount(() => (
<Button disabled>test</Button>
))
})
})
8 changes: 5 additions & 3 deletions packages/frontend-shared/src/components/Button.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
style="width: fit-content"
class="flex items-center leading-tight border rounded gap-8px outline-none"
:class="classes"
:disabled="disabled"
>
<ButtonInternals>
<template
Expand Down Expand Up @@ -33,7 +34,7 @@
</template>
</ButtonInternals>
</button>
<Component
<component
:is="internalLink ? BaseLink : ExternalLink"
v-else
:href="href"
Expand Down Expand Up @@ -68,7 +69,7 @@
</slot>
</template>
</ButtonInternals>
</Component>
</component>
</template>

<script lang="ts">
Expand Down Expand Up @@ -119,6 +120,7 @@ const props = defineProps<{
suffixIconClass?: string
href?: string // will cause the button to render as link element with button styles
internalLink?: boolean
disabled?: boolean
}>()

const attrs = useAttrs() as ButtonHTMLAttributes
Expand All @@ -133,7 +135,7 @@ const classes = computed(() => {
variantClasses.value,
sizeClasses.value,
attrs.class,
(attrs.disabled && props.variant !== 'pending') ? 'opacity-50' : '',
(props.disabled && props.variant !== 'pending') ? 'opacity-50' : '',
]
})
</script>
6 changes: 0 additions & 6 deletions packages/frontend-shared/windi.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ export const defaultConfig: WindiCssOptions = {
fontFamily: {
mono: '"Fira Code", monospace',
},
gridTemplateColumns: {
launchpad: '64px 1fr',
},
gridTemplateRows: {
launchpad: '64px 1fr',
},
colors,
},
},
Expand Down