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

Refactor obs form with collapsible panels #2308

Merged
merged 30 commits into from
Aug 17, 2024

Conversation

nimmolo
Copy link
Contributor

@nimmolo nimmolo commented Aug 15, 2024

#@mo-nathan @JoeCohen Please let me know if this solves it.

I tried including the naming fields in the top panel, but I didn't like them up there — it made it feel again like a barrage of too many fields all at once. This is now more like the iNat form: the identification and specimen stuff is right there above the "Create" button, but it has its own panel.

Screen Shot 2024-08-14 at 10 43 08 PM

The "collapsed"ness of each panel, when the form is loaded or reloaded, is conditional upon whether there are form-validation-feedback messages present in the panel. If there are messages ("below"), the panel should now display them.

This PR is in progress, but I'd ideally like to get feedback before I clean it up.

@coveralls
Copy link
Collaborator

coveralls commented Aug 15, 2024

Coverage Status

coverage: 93.484% (-0.04%) from 93.52%
when pulling 1148614 on refactor-obs-form-with-collapsible-panels
into 2cb2c1d on main.

@JoeCohen
Copy link
Member

This is great for typical user on a mobile device.
But there are trade-offs, in particular clutter vs clicking/scrolling.
For my workflow, I almost always add a fungus Name.
@mo-nathan:
(NOT something for this PR, but it's on my mind.)
What about foray recorders (who are using MO to enter all foray collections)?
They typically have a laptop or larger screen. And they almost always enter a Name.
And this happens hundreds of times daily for several days.
Saving one click or a bit of scrolling adds up.
Should there be different page for them, perhaps everything visible without clicking or scrolling?
Perhaps that can be done by compressing things (less white space), shortening text, using abbreviations, etc.

@nimmolo
Copy link
Contributor Author

nimmolo commented Aug 15, 2024

What about just starting the page with Identification flipped open? I think that will do it.

Screen Shot 2024-08-15 at 2 07 11 PM

@nimmolo nimmolo marked this pull request as ready for review August 15, 2024 21:12
@mo-nathan
Copy link
Member

mo-nathan commented Aug 15, 2024

@JoeCohen If recorders are using field slips it's not a problem (assuming the create observation form not getting the locality info from the field slip form is fixed). If they aren't then they'll want the name and notes sections open.

@nimmolo I agree we should work to minimize the number of empty fields that are visible and hide anything that is not yet relevant. But we need to balance that with showing all data that is automatically filled in. If this were the case, then when a new user clicks "Create Observation" it would just show the "Image / Select or drop files...", today's date, the locality field, and the scientific name field. Everything else would be hidden. If they enter the Locality then the "Is this location..." checkbox shows up. If they drop an image on the page that has lat/long then the locality is derived, and a lat/long panel opens that also has the Show map, Clear location, and Hide exact coordinates widgets.

The Scientific Name field would be shown, but the confidence and checkboxes would not show up until a name is provided. I think it makes sense for the "Specimen Available" checkbox to always be visible. Leaving it next to the Scientific Name seems reasonable, but it honestly could be it's own sub-panel (since it kind of acts that way already).

The Notes panel would be open if there is Notes data (from a Field Slip) or if they have an "Observation Notes Template" set. (As a side note, I wonder if we should make it easy to default to a reasonable subset of the standard Field Slip template to promote consistency. At least adding those as an example to the help text would be handy. I also wonder if the UI should let folks add fields if they want them, but that's definitely feature creep for this PR.)

I think the Project and Species List panels should be open if the user has any of them checked automatically (from previous usage or the field slip form).

I would also prefer it if in general the name of the panel toggled the panel and the V was next to the name of the panel. Otherwise I think we risk folks not being aware that they are subpanel and being confused by these words that don't seem to do anything.

I know this is a bunch more work, but it seems like it would be slickest way to handle this while making a sensible balance between keeping the UI tight and ensuring that users see all the stuff they want/need to see before clicking "Create".

@mo-nathan
Copy link
Member

@nimmolo I note that your image above implies that the sub-panel names are in a clickable bar with the V. However, when I run it locally that bar is not clickable and it's hardly visible from the "Agaricus" theme.
Screenshot 2024-08-15 at 7 49 33 PM

@nimmolo
Copy link
Contributor Author

nimmolo commented Aug 16, 2024

I'm busy debugging my refactor, but a lot of these seem sensible.

The one that doesn't make sense to me is "open the notes panel if they (even) have a notes template". Jason's notes template takes up the entire page, and I can't imagine he uses it all the time. (But maybe he does use it most of the time, it wouldn't be hard to find out.)
Screen Shot 2024-08-15 at 5 09 20 PM

@mo-nathan
Copy link
Member

My argument for showing them is that the user has control over the template and they went out of their way to include those fields as part of the form. I would guess that the vast majority of users who have any fields just have a handful of them. I agree it would be interesting to get @pellaea's opinion, but I'm guessing he won't really care and may even prefer to have it open. I know as a user of fields template, I would rather it was open by default than closed.

@nimmolo
Copy link
Contributor Author

nimmolo commented Aug 16, 2024

Ok, considering the suggestions more thoroughly. Action items:

  • Panel title shows/hides the panel OK
  • Hide confidence/reasons unless naming given Agree
  • Naming/Specimen in two columns - this is the way it is for layout reasons. The naming and specimen fields were way too wide on a desktop computer when I tried them in separate panels. For aesthetic reasons i don't want to stack panels that are different widths, so these two seemed like the best roommates to share a panel. They're not a perfect match, though, I agree.
  • Notes if any prefilled - agree. What am i checking for, though? @observation.notes.any? @field_slip.notes.any?
  • Projects/Lists open if any checked - should already will soon be the case
  • Only show geocode fields if they drop an image that has gps.
    This one I'm not in favor of, for three reasons:
    • You can click on the map to provide point data for any obs, manually, and because of that you need some visual indication that this is both possible and that it worked. I think we need an instruction there to encourage this. I will add it and we can see if that helps.
    • Some people input their own GPS textually, from some other source (e.g., @pellaea)
    • I think we want to encourage point data as much as possible.

@mo-nathan
Copy link
Member

mo-nathan commented Aug 16, 2024

  • Naming/Specimen in two columns - fine with me.
  • Notes if any prefilled - @observation.notes.present? seems to do the trick. There are examples of both NULL and empty serialized hashes. any? fails on nil.
  • Projects/Lists open if any checked It's not opening by default for me.
    Initial panel state:
Screenshot 2024-08-15 at 8 38 05 PM After I open the Projects panel: Screenshot 2024-08-15 at 8 38 14 PM
  • Only show geocode fields if they drop an image that has gps I think at least 80-90% of folks will rarely if ever manually enter lat/long info. Maybe a reasonable compromise is to open the panel if an image is provided regardless of whether there is lat/long info. I think it will be very rare for folks to want to provide lat/long info if they don't have a photo and if they do, it's just a click away. My goal is to keep new users focused on what needs to be done. For most users, they will start with dropping an image and then "magically" have their lat/long info appear with a checkbox they can click to obscure that info and that will feel good. Users who put in images that don't have lat/long will also be encouraged to provide the data if they have it since the empty fields will appear.

@nimmolo
Copy link
Contributor Author

nimmolo commented Aug 16, 2024

re: projects, yep i had missed that. I just pushed that change.

locality and lat/lng in separate panels - Not really feasible considering the complexity of all the JS i just did: the locality (not just the lat/lng) interacts with, and depends upon, the map, particularly the whole "Create location" flow.

I can just make a "show geolocation" checkbox to show hide the lat/lng, and show by JS if there are images.

@mo-nathan
Copy link
Member

Are you saying, put a checkbox next "Geolocation" (and maybe change it to "Show geolocation" which can hide or show this bit:
Screenshot 2024-08-15 at 9 28 33 PM
But it would get checked automatically if there are images or the map gets shown? If that's right, that sounds great!

@JoeCohen
Copy link
Member

JoeCohen commented Aug 16, 2024 via email

@JoeCohen
Copy link
Member

JoeCohen commented Aug 16, 2024 via email

@JoeCohen
Copy link
Member

Thanks for the continued improvements. (Images + Details accordion-able; more things collapsed on Edit Obs.)

Create Obs Location/Geolocation still presents some issue for me.
Here's what I see after Create Observation, dragging a photo to Images + Details, clicking on the Geolocation ﹖icon:
Screenshot 2024-08-17 at 3 02 44 AM

The photo lat/lng is a long way from Deschutes Co., which is the Locality of my last-created Obs. That Obs was last created 23 Jul.
Issues/Suggestions:

  • The old default Location shouldn't persist so long. I think in the past it persisted only an hour or so.
    @mo-nathan: what do you think?
  • I don't see how to use your nifty Locality picker. (How do I get it to use the MBR as the default, with the dropdown list including only other bounding Localities?)
  • if lat/lng are present, hide the Geolocation checkbox, label, and help icon.

@nimmolo
Copy link
Contributor Author

nimmolo commented Aug 17, 2024

Fixed Joe's last issue.

The issue was that I changed the HTML nesting around the locality autocompleter, to get the geolocation fields to be collapsible. But I had not moved the HTML ID.

The ID needs to be on the outermost div that calls the Stimulus controller, to correctly connect to it. It was simply disconnected.

@nimmolo nimmolo merged commit c0ba874 into main Aug 17, 2024
5 of 8 checks passed
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.

4 participants