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

Add stepper-layout prop to OdkWebForm component for Collect-like experience #329

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

spwoodcock
Copy link

@spwoodcock spwoodcock commented Mar 10, 2025

I have verified this PR works in these browsers (latest versions):

  • Chrome
  • Firefox
  • Safari (macOS)
  • Safari (iOS)
  • Not applicable

What else has been done to verify that this works as intended?

  • Tested the original 'list' view layout works too.

Why is this the best possible solution? Were any other approaches considered?

  • I tried my best to not add new superfluous code.
  • It's mostly either:
    • Using built-in PrimeVue components.
    • Using existing functionality built into web-forms already (such as the validation code).

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

  • No change for the default view - the stepper has to be enabled by a prop.

Do we need any specific form for testing your changes? If so, please attach one.

I was testing with this one:
mdp_fmtm_xlsform.xlsx

What's changed

Included

  • I added a new component QuestionStepper that mirrors the QuestionList component.
  • It's enabled by simply passing the stepper-layout prop to OdkWebForm.
  • Instead of displaying questions and groups via collapsible panels, instead a PrimeVue Stepper component is introduced, displaying one question or group of questions per page.
  • I kind of abused the submitPressed dependency to trigger the existing validation logic, without needing to add extra code - it seems to work nicely.
  • Video is in desktop view for easier recording, but mobile view is the preferred way to use the stepper:
Recording.2025-03-10.134548.mp4

Things I didn't do

  • I updated the FormPanel to have an optional toggleable prop. This allows us to disable the panel collapsing behaviour for the stepper. I didn't update RepeatInstance and RepeatRange to have a toggleable prop passed through to FormPanel too, but assume this may be needed?
  • I attempted to add a PrimeVue ProgressBar to the stepper. It works well in the layout, however, it's pretty tricky to assess the percentage complete for the form. This needs to be done via the available nodes, but based on relevant fields, additional nodes can be added causing the percentage complete to drop. I need to look into this in more detail - it's probably doable - if this is a desired feature.
  • I had to hide the 'Powered by ODK' button when using the stepper, as it has absolute positioning below the question panel. This could be fixed somehow if needed.

Note on the FormPanel border

  • I removed this entirely for the stepper based view, as it's not necessary imo:
    Stepper view:
    image
    List view:
    image
  • I kept the left border on collapsible FormPanel as it nicely demarcates where each group starts and ends.
    • I assume that was why it is present? Else, easily removable via box-shadow CSS.

Extra notes

Copy link

changeset-bot bot commented Mar 10, 2025

⚠️ No Changeset found

Latest commit: 0d0363c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@spwoodcock spwoodcock force-pushed the feat/submission-stepper branch from 3579ece to 0d0363c Compare March 10, 2025 13:58
@@ -48,7 +48,7 @@ const isControlNode = (node: NonStructuralNode): node is ControlNode => {
<template v-for="node in nodes" :key="node.nodeId">
<template v-if="node.currentState.relevant">
<!-- Render group nodes -->
<FormGroup v-if="isGroupNode(node)" :node="node" />
<FormGroup v-if="isGroupNode(node)" :node="node" toggleable />
Copy link
Author

Choose a reason for hiding this comment

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

I set the default for the prop toggleable to false, meaning the prop needs to be passed here to allow for the collapsable panels in the list view

@@ -140,6 +151,11 @@ h2 {
}
}

.toggleable-enabled :deep(.p-panel-content) {
Copy link
Author

Choose a reason for hiding this comment

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

Here I only add the style for the line on the left of the group if the panel is set to toggleable

Screenshot 2025-03-10 135310

<template>
<Panel v-if="!noUi" :class="panelClass" :toggleable="true" :collapsed="panelState">
<Panel v-if="!noUi" :class="[panelClass, { 'toggleable-enabled': toggleable }]" :toggleable="toggleable" :collapsed="toggleable ? panelState : false">
Copy link
Author

Choose a reason for hiding this comment

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

Adding the toggleable-enabled class dynamically

@lognaturel
Copy link
Member

Thanks so much for exploring this and sharing what you've come up with! The video looks so good and it's great to see that the implementation is largely contained.

I'm going to change this to a draft for now to help with our review processes. We'd want to make sure there are tests and a design pass before considering for merge.

I think it makes sense for us to prioritize image capture and selects with images over reviewing and finalizing this, does that sound right?

@lognaturel lognaturel marked this pull request as draft March 10, 2025 16:14
@spwoodcock
Copy link
Author

Thanks so much for exploring this and sharing what you've come up with! The video looks so good and it's great to see that the implementation is largely contained.

I'm going to change this to a draft for now to help with our review processes. We'd want to make sure there are tests and a design pass before considering for merge.

I think it makes sense for us to prioritize image capture and selects with images over reviewing and finalizing this, does that sound right?

Yep sounds good to me! We can finalise and discuss this further in the long term - I'll add tests too 😁

We can use this layout on our fork for the time being 👍

@alyblenkin
Copy link
Collaborator

@spwoodcock this is awesome! I'm working on designing the flows for image capture and selects with images. I'll share it with you once I have something ready to get your feedback.

@spwoodcock spwoodcock mentioned this pull request Mar 24, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants