Closed tillwenke closed 2 weeks ago
This looks very good!
Do you know how the details element remembers its open/closed state? Should we add it to LocalStorage just to be sure?
Although we don't store it, most browsers remember the
1. Blank out the select element on reload. Not my favorite choice.
2. Always expand the details element if the select value is already set, to show the user. Seems good now, might get annoying if we add a lot of questions.
3. Move the signal out of the details element, leaving only gender info. Also not my favorite choice.
4. No details element, break the form in two parts, with a button "Submit summary review" after the first part and a button "Submit extended review" after the extended questions. Might be confusing because web forms are rarely structured this way.
didnt find anything about that the open state was remembered. also tested on some sites where it got back to its initial state when reloading the site.
I chose 2. bc it encourages to add the extra info and prevents the user from forgetting that some values were set already.
could open an issue to never remember the signal state.
I could have sworn the element remembered to stay closed when I tested it earlier today. I don't think leaving it open at all times makes sense. If it's always open when the form is shown, users might as well scroll past it instead of collapsing it.
How about this:
could open an issue to never remember the signal state.
More often than not, this remains the same though
I've been thinking about this and I'm kinda worried about making reviews feel like a questionnaire. Rereading #37, that's the reason I originally suggested this feature, and why I originally suggested it'd be collapsed by default.
I'm leaning that way again.
The extended form is collapsed by default, it optionally remembers state, and if the form is submitted and the extended form has never been shown but it has previously entered data, the user is asked to confirm.
I'll only merge this once we actually have multiple form inputs we want to collapse, so you can also commit the hh-date stuff you're working on straight to this PR, and I'll merge them in one go.
how do we go about new column in the db? could you add "datetime_ride" to it?
Ready to merge @bopjesvla Decided to just force to look at the collapsed items before submitting if values are set there.
Checking if it was open at some point was too complex.
I updated the styling to make the main flow more apparent.
thanks for the review. can we merge now? @bopjesvla do we have the datetime_ride column in the database?
I don't think you correctly merged the master branch; the new sidebar is gone for example.
Done :) You can merge @bopjesvla
I've added the column in the db. Two more things:
"When did you get the ride?"
Reviewers don't always get a ride, and you might still want the time of day they hitchhiked. I can't really think of one natural way to ask this, so I'd go with the current option if they logged a destination (i.e. points[1].lat is a number) and add something like "If you didn't get a ride, when did you stop soliciting rides?" if not.
I'd recommend using classList.toggle on the spot form and hiding the addition using CSS
You can also replace the datetime with the ride_datetime if it exists, and add an icon to show it's not the time of review. Something like 🕒 November, 2006 instead of --November, 2024.
download function in a separate pr please
and you can just make a .sh with the download line in the README
There's a change to the dashboard as well. Again, remember to check the diffs before committing.
looks good now imo
done
@bopjesvla how do we know people didnt get a ride? wanna open another issue for this?
@bopjesvla how do we know people didnt get a ride? wanna open another issue for this?
There used to be an option for people who didn't get a ride, but it was misused to skip (even though there was also a skip option). Bad rating + a skip is probably the best way to tell
For futurereference: while the signal input was on the main form, 2906/3558 reviewers filled it in.
collapses optional information that we want to record for spots. we can put future features like gender there as well to not distract the average user.
partially solves #37
expanded by default so that one can see which values were cached from before and to encourage filling out these optional fields. opens #60