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.

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

feat #128: first step towards responsive layout #132

Merged
merged 6 commits into from
Jun 26, 2024
Merged
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
5 changes: 5 additions & 0 deletions .changeset/128-responsive-layout-init.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@getodk/web-forms": patch
---

Responsive layout: Adjust Form header based on screen size (#128)
6 changes: 3 additions & 3 deletions packages/web-forms/e2e/vue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ test('All forms are rendered and there is no console error', async ({ page, brow
// this ensures that Vue application is loaded before proceeding forward.
await expect(page.getByText('Demo Forms')).toBeVisible();

const forms = await page.getByText('Show').all();
const forms = await page.locator('ul.form-list li').all();

expect(forms.length).toBeGreaterThan(0);

Expand Down Expand Up @@ -73,14 +73,14 @@ test('All forms are rendered and there is no console error', async ({ page, brow
}
}

const langChanger = page.getByLabel('change language');
const langChanger = page.locator('.larger-screens .language-changer');

if ((await langChanger.count()) > 0) {
await langChanger.click();
await page.locator('.language-dd-label').last().click();
}

await page.getByText('Back').click();
await page.goBack();
}

expect(consoleErrors).toBe(0);
Expand Down
38 changes: 35 additions & 3 deletions packages/web-forms/icomoon.json
Original file line number Diff line number Diff line change
Expand Up @@ -488,19 +488,51 @@
"setIdx": 0,
"setId": 4,
"iconIdx": 35
},
{
"icon": {
"paths": ["M128 256h768v86h-768v-86zM128 554v-84h768v84h-768zM128 768v-86h768v86h-768z"],
"attrs": [],
"isMulticolor": false,
"isMulticolor2": false,
"tags": ["menu"],
"grid": 24
},
"attrs": [],
"properties": {
"ligatures": "menu",
"id": 600,
"order": 299,
"prevSize": 48,
"code": 59670,
"name": "menu"
},
"setIdx": 0,
"setId": 4,
"iconIdx": 600
}
],
"height": 1024,
"metadata": { "name": "icomoon" },
"metadata": {
"name": "icomoon"
},
"preferences": {
"showGlyphs": true,
"showQuickUse": true,
"showQuickUse2": true,
"showSVGs": true,
"fontPref": {
"prefix": "icon-",
"metadata": { "fontFamily": "icomoon", "majorVersion": 1, "minorVersion": 0 },
"metrics": { "emSize": 1024, "baseline": 6.25, "whitespace": 50 },
"metadata": {
"fontFamily": "icomoon",
"majorVersion": 1,
"minorVersion": 0
},
"metrics": {
"emSize": 1024,
"baseline": 6.25,
"whitespace": 50
},
"embed": false,
"showSelector": true,
"showMetrics": true,
Expand Down
57 changes: 49 additions & 8 deletions packages/web-forms/src/OdkWebFormDemo.vue
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,66 @@ const handleSubmit = () => {
alert(`Submit button was pressed`);
}

const showForm = (form: [string, string]) => {
selectForm.value = form;
history.pushState({form: form }, "", "/" + form[0]);
}

interface PopStateEventWithForm extends PopStateEvent {
state: {form: [string, string]};
}

window.addEventListener("popstate", (event:PopStateEventWithForm) => {
if(!event.state) {
selectForm.value = null;
}
else {
selectForm.value = event.state.form;
}
});

if(location.pathname != '/'){
const demoForm = demoForms.find(f => `/${f[0]}` === location.pathname) ?? null;
if(demoForm) {
selectForm.value = demoForm;
}
else{
history.replaceState(null, "", "/");
}
}
</script>

<template>
<div v-if="!selectForm">
<h1>Demo Forms</h1>
<ul>
<li v-for="form in demoForms" :key="form[0]">
<ul class="form-list">
<li v-for="form in demoForms" :key="form[0]" @click="showForm(form)">
{{ form[0] }}
<button @click="selectForm = form">
Show
</button>
</li>
</ul>
</div>
<div v-else>
<button @click="selectForm = null">
Back
</button>
<OdkWebForm v-if="selectForm" :form-xml="selectForm[1]" @submit="handleSubmit" />
</div>
</template>

<style>
ul.form-list {
padding: 0;

li {
list-style: none;
padding: 10px;
margin: 10px;
border: 1px solid var(--primary-500);
border-radius: 10px;
cursor: pointer;
background-color: var(--surface-0);
}

li:hover {
background-color: var(--primary-50);
}

}
</style>
3 changes: 3 additions & 0 deletions packages/web-forms/src/assets/css/icomoon.css
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,6 @@
.icon-repeat:before {
content: '\e914';
}
.icon-menu:before {
content: '\e916';
}
2 changes: 1 addition & 1 deletion packages/web-forms/src/assets/css/style.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
body {
background: var(--gray-200);
margin: 0;
}
1 change: 1 addition & 0 deletions packages/web-forms/src/assets/fonts/icomoon.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified packages/web-forms/src/assets/fonts/icomoon.ttf
Binary file not shown.
Binary file modified packages/web-forms/src/assets/fonts/icomoon.woff
Binary file not shown.
126 changes: 120 additions & 6 deletions packages/web-forms/src/components/FormHeader.vue
Original file line number Diff line number Diff line change
@@ -1,20 +1,115 @@
<script setup lang="ts">
import Card from 'primevue/card';
defineProps<{title: string}>();
import { type FormLanguage, type RootNode, type SyntheticDefaultLanguage } from '@getodk/xforms-engine';
import PrimeButton from 'primevue/button';
import PrimeCard from 'primevue/card';
import PrimeMenu from 'primevue/menu';
import { ref } from 'vue';
import FormLanguageDialog from './FormLanguageDialog.vue';
import FormLanguageMenu from './FormLanguageMenu.vue';

const props = defineProps<{form: RootNode}>();
const languageDialogState = ref(false);
const menu = ref<PrimeMenu>();

const isFormLanguage = (lang: FormLanguage | SyntheticDefaultLanguage) : lang is FormLanguage => {
return !lang.isSyntheticDefault;
}

const languages = props.form.languages.filter(isFormLanguage);

const print = () => window.print();

const items = ref([
{
label: 'Print',
icon: 'icon-local_printshop',
command: print
}
]);

if(languages.length > 0){
items.value.unshift({
label: 'Change language',
icon: 'icon-language',
command: () => languageDialogState.value = true
})
}

const handleLanguageChange = (event: FormLanguage) => {
props.form.setLanguage(event);
};
</script>

<template>
<Card class="form-title">
<!-- for desktop -->
<div class="hidden lg:flex justify-content-end flex-wrap gap-3 larger-screens">
<PrimeButton class="print-button" severity="secondary" rounded icon="icon-local_printshop" @click="print" />
<FormLanguageMenu
:active-language="form.currentState.activeLanguage"
:languages="languages"
@update:active-language="handleLanguageChange"
/>
</div>
<PrimeCard class="form-title hidden lg:block">
<template #content>
<h1>{{ title }}</h1>
<!-- TODO/q: should the title be on the definition or definition.form be accessible instead of definition.bind.form -->
<h1>{{ form.definition.bind.form.title }}</h1>
<!-- last saved timestamp -->
</template>
</Card>
</PrimeCard>

<!-- for mobile and tablet -->
Copy link
Member

Choose a reason for hiding this comment

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

Also not something we need to act on right now (unless you want to explore it!): if we are sure we want to preserve the single-row presentation and condense horizontal space, this feels like a potentially excellent use case for container queries. Using it well for this might have other design implications (i.e. we'd need to know how much space is needed upfront before we can query it).

The nice thing about container queries here would be the possibility of triggering space-saving as needed, which might allow more functionality to stay visible for e.g. forms with shorter titles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion. For now I have run with the existing media query, I will try to use it next time we have any changes in the UI of the Form header.

<div class="flex lg:hidden align-items-center smaller-screens">
Copy link
Member

Choose a reason for hiding this comment

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

I was curious to see how wrapping looks for forms with long titles. Pretty good! I think we may want to iterate later on...

  • More consistent alignment of controls (icons in larger views, hamburger menu in smaller). The vertical centering makes those a moving target. I think we'll probably want some design guidance on that.

  • Maybe slightly reducing text size/weight over a certain character length? (Honestly the weight feels pretty heavy regardless of length, wrapping.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points, I am quoting these in #139

<h1 class="flex-grow-1">
{{ form.definition.bind.form.title }}
</h1>

<!-- for tablet -->
<div class="form-options hidden md:flex justify-content-end gap-3">
<PrimeButton class="print-button" severity="secondary" rounded icon="icon-local_printshop" @click="print" />
<FormLanguageMenu
:active-language="form.currentState.activeLanguage"
:languages="languages"
@update:active-language="handleLanguageChange"
/>
</div>

<!-- for mobile -->
<div class="form-options flex md:hidden">
<PrimeButton v-if="languages.length > 0" icon="icon-menu" class="btn-menu" text rounded aria-label="Menu" @click="menu?.toggle" />
<PrimeButton v-else class="print-button" severity="secondary" rounded icon="icon-local_printshop" @click="print" />
<PrimeMenu id="overlay_menu" ref="menu" :model="items" :popup="true" />
<FormLanguageDialog
v-model:state="languageDialogState"
:active-language="form.currentState.activeLanguage"
:languages="languages"
@update:active-language="handleLanguageChange"
/>
</div>
</div>
</template>

<style scoped lang="scss">
.p-button.p-button-icon-only.p-button-rounded {
height: 2.5rem;
width: 2.5rem;
min-width: 2.5rem;
font-size: 1.5rem;

&:hover{
background: var(--primary-100);
}
&:active, &:focus {
background: var(--primary-50);
}
Comment on lines +99 to +104
Copy link
Member

Choose a reason for hiding this comment

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

Unclear where to put this comment, but this is where it's most visible: the hamburger menu looks great in its default state, but when these background colors are applied it shows the background all the way to the right edge of the viewport. This is a minor nitpick, but it feels a little unpolished and maybe worth a little finesse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a discussion with Aly and she suggested to add equal spacing on the right side as there is on the left side, which is 10px.

Copy link
Member

Choose a reason for hiding this comment

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

Now that you mention this, it caught my eye that quite a lot of measurements in the styles are in px. I don't think we should worry about this now, but I would like to give some thought into where relative units make sense for accessibility/scaling flexibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's talk about this in Slack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We talked about using rem instead of px in Slack and setting different font-size of root for different screen sizes. Tracking it in #143

}


.form-title {
// var(--light-elevation-1);

border-radius: 10px;
box-shadow: 0px 1px 3px 1px #00000026;
box-shadow: var(--light-elevation-1);
border-top: none;
margin-top: 20px;

Expand All @@ -28,4 +123,23 @@ defineProps<{title: string}>();
}
}
}

.smaller-screens {
background-color: var(--surface-0);
filter: drop-shadow(0px 2px 6px rgba(0, 0, 0, 0.15)) drop-shadow(0px 1px 2px rgba(0, 0, 0, 0.30)) ;

h1 {
padding-left: 10px;
font-size: 1.5rem;
}

.form-options{
padding-right: 10px;
}

.btn-menu{
color: var(--surface-900);
}
}

</style>
Loading