Skip to content

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

Draft
wants to merge 58 commits into
base: form_maker
Choose a base branch
from
Draft

Implement Row Seeker within Forms #4637

wants to merge 58 commits into from

Conversation

seancolsen
Copy link
Contributor

@seancolsen seancolsen commented Jul 15, 2025

Stacked on #4609

Fixes #4636

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the develop branch of the repository
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@seancolsen
Copy link
Contributor Author

This PR is a continuation of @pavish's work in #4556.

My overall steps from this point are:

  1. Integrate the row seeker into the form page.
  2. Make the row seeker API accept a form id so that it work for anonymous users.
  3. Revert changes to table page and record page, using the record selector there again.
  4. Make small UX improvements to the row seeker, given time constraints and priorities
  5. Work with Kriti and Zack to get the final UX approved.
  6. Work with Brent and Anish to get the backend changes approved.

The small UX improvements I'm considering are as follows:

  • Improve initial loading state and dropdown positioning
  • Need text truncation for long record summaries. E.g. search for movie "worry" with narrow viewport.
  • Improve UX for selected row (reduce confusion with "in focus" row)
  • Use custom styling for "No records found"
  • Use custom styling for "Loading" text, or a spinner
  • Style the footer differently from the options
  • Move the pagination to the right of the footer (to match record selector)

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.

@seancolsen seancolsen added this to the v0.5.0 milestone Jul 16, 2025
@seancolsen
Copy link
Contributor Author

seancolsen commented Jul 16, 2025

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:

image

And when there is no previously-selected value, then no radio inputs display.

I hope this is okay with you.

@seancolsen
Copy link
Contributor Author

@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.

@seancolsen seancolsen added the pr-status: review A PR awaiting review label Jul 24, 2025
@seancolsen
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-status: review A PR awaiting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants