-
-
Notifications
You must be signed in to change notification settings - Fork 384
Implement Row Seeker within Forms #4637
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
base: form_maker
Are you sure you want to change the base?
Conversation
This PR is a continuation of @pavish's work in #4556. My overall steps from this point are:
The small UX improvements I'm considering are as follows:
But ahead of the UX improvements, I'll be prioritizing higher-level integration with the forms system so that we can get to the forms MVP ASAP. |
I pushed some commits which address all of the small UX improvements I had in mind. Most of the changes are straightforward, but @pavish there's one I wanted to call out because we didn't discuss it on our call... I found the UX to be slightly confusing when the row seeker is opened with a previous selection. As you pointed out on the call, this case might happen in the context of forms if the user chooses a value and then decides to change it. In this state, there is a difference between the "selected" row (the one matching the previously-chosen value) and then "in focus" row (the one the user is about to choose via the keyboard movements). You had set this up to use an orange background for selected and a grey background for in-focus. When I played with this initially I didn't understand the difference between orange and grey. I changed it so that when there is a previously-selected value, each row shows a radio input like this: ![]() And when there is no previously-selected value, then no radio inputs display. I hope this is okay with you. |
Can't access svelte context outside component initialization.
@pavish I think this PR is very close to some sort of "done" state now. I still have a small amount of tidying to do on the backend (write SQL tests, fix the pytest failure, document the new API). But on the front end I'm not currently seeing anything else needed. So in parallel with my backend tidying, I'd like you to review the front end changes when you get a chance. I did a small amount of QA, and I tried with nested form elements and such. It seems to work well. But I wouldn't be surprised if something is amiss because I don't have the firmest grasp on the Forms system yet. Perhaps it's weird to review a draft PR, but I think it'll be useful. We'll probably need to chat in more detail at an upcoming meeting about what sort of review process we want to use to get all this work merged, because this is definitely non-standard for us. |
Before this fix VS Code's syntax highlighting was choking on this code
@mathemancer this is ready for backend review now. This PR in draft state because it's stacked on top of #4609, which is still in progress. So don't merge this. But if you could please review the backend code that would help move this along. |
Stacked on #4609
Fixes #4636
Checklist
Update index.md
).develop
branch of the repositoryDeveloper Certificate of Origin
Developer Certificate of Origin