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

Add shopping list items using the enter key #3118

Merged
merged 5 commits into from
Feb 19, 2024

Conversation

Fastjur
Copy link
Contributor

@Fastjur Fastjur commented Feb 5, 2024

What type of PR is this?

  • feature

What this PR does / why we need it:

  • Allows saving of shopping list items using the enter key
  • Multiline notes are still supported in shopping lists, using shift+enter
  • Also adds some simple frontend checks to prevent empty items from being added, as that has become a bit easier to accidentally do using the enter key.

Enter to save functionality:

Peek 2024-02-05 12-10

Prevent empty items from being added:

Peek 2024-02-05 12-11

Which issue(s) this PR fixes:

Related to: #3114

Fastjur and others added 4 commits February 5, 2024 11:17
… field

Related to: mealie-recipes#3114
* Enter key press is caught in note field in ShoppingListItemEditor
* The create editor now stays open after saving a food item to a shopping list,
   to allow keyboard-only interaction with the shopping list
Related to: mealie-recipes#3114
An item is considered empty when the foodId is not set, and no note is set.
This is only handled frontend, the backend still accepts empty items.
@michael-genson
Copy link
Collaborator

This looks good! A few notes after playing around with it a bit:

  • When adding by food, you have to tab back to the note for the enter to work
  • When adding a food, you have to press "down" and hit enter, even if there's only one result (this is true for all dropdowns)
  • If I want to add by food, I have to toggle food mode every time I create a new item. I would expect to only need to toggle once

I'm okay with merging as-is, since it's still a good improvement, but I think getting a few other changes in will make the workflow much better

@michael-genson
Copy link
Collaborator

Also, the latest merge fixes the food store bug you mentioned in discord, so it should be working as expected now

@Fastjur
Copy link
Contributor Author

Fastjur commented Feb 5, 2024

  • When adding by food, you have to tab back to the note for the enter to work
  • When adding a food, you have to press "down" and hit enter, even if there's only one result (this is true for all dropdowns)

Yeah, I have purposefully not touched the other fields as the keyboard can be used to navigate them, but I indeed do agree that they too are a bit clunky perhaps.
I'm happy to add other changes to this PR for those fixes, if there can be a consensus reached on what the expected behaviour is.

For example, when the food dropdown/textfield is focussed, what is the expected behaviour of an enter key? How about the quantity? The dropdowns, combined with their search bar and then using the arrow keys and enter to select have never really worked nicely in any dropdowns throughout mealie for me, though. Instead, usually when I select a food item and press enter, the field is not actually set to that food item but the default one it selected, so perhaps that is another issue altogether to tackle instead of in this PR.

  • If I want to add by food, I have to toggle food mode every time I create a new item. I would expect to only need to toggle once

This I can definitely add to this PR, it seems more logical to me anyway to increase the speed of the shopping list fields.

I'm okay with merging as-is, since it's still a good improvement, but I think getting a few other changes in will make the workflow much better

Agreed, if we can discuss a set of expected behaviour around this specific component I'd be happy to implement it in this PR.

@michael-genson
Copy link
Collaborator

when the food dropdown/textfield is focussed, what is the expected behaviour of an enter key? How about the quantity?

I would expect the enter key to universally create the item, with the exception of when the dropdown is open (e.g. while typing a food). In that instance, I would expect the enter key to select the highlighted food (or the first one, if there is only one food).

The dropdowns, combined with their search bar and then using the arrow keys and enter to select have never really worked nicely in any dropdowns throughout mealie for me, though

Yeah I agree, if you're able to figure that one out it would be nice, but might be out of scope for this PR.

I'm happy to be convinced otherwise on any of the above, I'm a backend guy mostly, but that's what my user-intuition is telling me.

@michael-genson
Copy link
Collaborator

I went ahead and implemented a few of the things discussed above in #3178; Happy to get this merged, unless you wanted to take a look and handling the enter key on other fields first

@Fastjur
Copy link
Contributor Author

Fastjur commented Feb 15, 2024

Ah sorry Michael. This slipped my mind. I can take a look tomorrow and move the enter event handler up a few levels until it works on the entire form. I’ll have to see to prevent it submitting in the case the dropdown is open. Otherwise I think an enter is always supposed to submit. I’ll add that to the PR tomorrow and let you know once it is updated. Should be a relatively easy fix too.

@Fastjur
Copy link
Contributor Author

Fastjur commented Feb 19, 2024

Sorry, wasn't able to work on it Friday. Starting on it right now :)

Signed-off-by: Jurriaan Den Toonder <1493561+Fastjur@users.noreply.github.com>
@Fastjur
Copy link
Contributor Author

Fastjur commented Feb 19, 2024

So, I did spend quite some time on this. However, I am not very used to Vue, not at all to be honest. If I want to handle enters in the dropdown InputLabelType components, I think I would have to alter them to have some checks on when to emit enter events and when not. I'm not sure how to read out if the menu is open, in the handleKeyPress handler in ShoppingListItemEditor. I have no idea how this is properly done in Vue, and I suspect it would require some changes to the InputLabelType component, which I'm not comfortable to do as I have no idea where this is all used.

I think it's better if someone with more Vue experience and experience in this code base would handle that

@Fastjur
Copy link
Contributor Author

Fastjur commented Feb 19, 2024

Additionally, as the enter key opens and closes the dropdown, I think adding logic to the generic component may break how people use these dropdowns. I honestly think a first change would be improving how these dropdowns work in the entirety of mealie, and then think about how we would handle enter keypresses in fields like these.

@michael-genson
Copy link
Collaborator

Makes sense to me, I had a feeling it wouldn't be an easy change, but I can dream. Thanks for taking a look anyway!

Is this ready? I'll give it a final lookover

@Fastjur
Copy link
Contributor Author

Fastjur commented Feb 19, 2024

This is ready, it has exactly the same functionality as in the gifs. It does not address the points you commented on initially. But I think you took care of one (or more) of them?

But yeah, at least in the textarea field (the note) it works now, IMO this already improves it greatly until there is a better solution for the generic input fields used accross mealie :)

Copy link
Collaborator

@michael-genson michael-genson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for your work on this!

@michael-genson michael-genson merged commit 4d2363e into mealie-recipes:mealie-next Feb 19, 2024
9 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.

None yet

2 participants