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

Cannot switch between new log types using the app shell menu #95

Closed
alexadamsmith opened this issue Feb 5, 2019 · 11 comments
Closed

Cannot switch between new log types using the app shell menu #95

alexadamsmith opened this issue Feb 5, 2019 · 11 comments

Comments

@alexadamsmith
Copy link
Collaborator

When I'm in the 'edit log' view, and log type is set to harvest, I can open the app shell menu and select 'new observation'. But when I do so, I just stay on 'edit log', with the log type selector still set to harvest. Of course I could just use the log type selector, but some users might still find this frustrating.

In an email exchange, @jgaehring said "We'll have to figure out a better way to set the params. There are two ways to do it: you can set the params property, which works kind of like props, or you can set the query property, which actually adds query params to the urls (eg, '/logs/edit?type=observation`). We're currently using the first approach, but perhaps the latter approach would fix the issue?"

@alexadamsmith
Copy link
Collaborator Author

alexadamsmith commented Feb 6, 2019

I changed the way we are handling params for the edit logs view. By setting the params as part of the view route, the params can be accessed directly as props. And with 'type' as a prop, I could set a watcher on it and update the log type whenever 'type' changed!
Here's a tutorial for this way of handling routes

I also re-wrote our type selector so that it reads from a logTypes object, rather than having the types hard-coded into the template.

@jgaehring
Copy link
Member

Oh great! I'll have to dive a little deeper into the docs and study it alongside your commits, but this looks like a definite improvement on how we were doing it. Good detective work, @alexadamsmith !

@jgaehring
Copy link
Member

jgaehring commented Feb 7, 2019

Hey, @alexadamsmith, I'm playing around with these changes now, and it looks great, I really like the way you've parameterized the type!

However, I'm seeing that when you select "New [log type]" from the app drawer, it just changes the type of the current log type. I feel like the expected behavior would be to create a new log with the new type. Was this the behavior you intended?

I'm thinking instead of the watcher on the type prop, we could set a watcher on the $route object itself, and then just duplicate the behavior we use with the created() hook (see the Vue Router docs on Reacting to param changes). That would achieve the latter behavior, which seems more intuitive to me.

Everything else looks great though!

@alexadamsmith
Copy link
Collaborator Author

alexadamsmith commented Feb 7, 2019 via email

@jgaehring
Copy link
Member

Cool, Yea, I think I'll change it. I think the idea should be that the log is always saved, and they can always come back and edit it later. But when they click "New Harvest", that's signaling that they're done editing it for now, and are creating a new harvest log.

I guess I could see it both ways though. @mstenta, what's your view?

@jgaehring
Copy link
Member

I submitted a PR (#42) with the proposed change.

@mstenta
Copy link
Member

mstenta commented Feb 7, 2019

But when they click "New Harvest", that's signaling that they're done editing it for now, and are creating a new harvest log.

I think this seems right to me. The word "New" in there implies a new log will be started. So if you have another one open in the editor, it should be saved as-is so you can come back to it later if you need.

@mstenta
Copy link
Member

mstenta commented Feb 7, 2019

Since the user hasn't selected 'done editing' yet, I didn't want to assume they were finished.

This makes sense too - but perhaps we can change "Done editing" to just "Save" now? Earlier we didn't have the ability to edit logs after you clicked "Done editing" so it was more final. Now we can edit them, so maybe "Save" would imply less commitment.

@jgaehring
Copy link
Member

This makes sense too - but perhaps we can change "Done editing" to just "Save" now? Earlier we didn't have the ability to edit logs after you clicked "Done editing" so it was more final. Now we can edit them, so maybe "Save" would imply less commitment.

I'm not sure that I like "Save" because that implies that the log is not saved until you hit that button, when it's actually saved the moment you type or enter input any other way. All hitting that button does, literally, is take you back to the "All Logs" page. I think an ideal flow is that they can use the drawer to create log after log after log, without ever having to navigate back to the "All Logs" display. I could see that being useful out in the field. That was partly why we went with "Done Editing" as opposed to "Saved". Perhaps a better choice would be "Back to Logs" since that is more literally what that button is doing?

@mstenta
Copy link
Member

mstenta commented Feb 8, 2019

That was partly why we went with "Done Editing" as opposed to "Saved".

That makes sense. I'd say lets just keep it as-is for now then. "Done editing" works.

@mstenta mstenta transferred this issue from farmOS-legacy/farmOS-client Feb 19, 2019
@mstenta
Copy link
Member

mstenta commented Feb 19, 2019

(Transferring all issues from old repository. See #92)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants