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

Date picker changes #304

Merged
merged 5 commits into from
Sep 17, 2021
Merged

Conversation

ntrecina
Copy link
Contributor

@ntrecina ntrecina commented Sep 6, 2021

Summary

I also noticed a regression from 0a74061 where the readonly attribute was removed so mobile browsers were once again displaying the keyboard when tapping on the date picker field. I added the inputmode=none attribute which seems like it might be a better solution while still allowing the field to be editable via a keyboard if desired.

…ilk as the type. Also added inputmode=none attribute to all date pickers to prevent keyboard from opening on mobile browsers
@coveralls
Copy link

coveralls commented Sep 6, 2021

Coverage Status

Coverage decreased (-0.1%) to 99.163% when pulling f504cd3 on ntrecina:Feeding_and_date_picker_changes into c8d4894 on babybuddy:master.

@cdubz cdubz temporarily deployed to babybuddy-pr-304 September 6, 2021 11:59 Inactive
@cdubz
Copy link
Member

cdubz commented Sep 7, 2021

@ntrecina what do you think of the discussion in #284? I think it’s reasonable to allow direct modification of the datetime fields for certain use cases and the UX is not necessarily bad with the keyboard appearing on mobile.

@cdubz
Copy link
Member

cdubz commented Sep 7, 2021

Re: the static files — could you see if you get the same result from gulp updatestatic? I don’t think we should be generating FA fonts anymore. Unless they are used in Django admin?

@ntrecina
Copy link
Contributor Author

ntrecina commented Sep 8, 2021

@cdubz I think it makes sense to have direct modification of date/time fields as mentioned in #284 as I have run into some of the scenarios mentioned. However, I may be wrong but I think the 99% use case on mobile will involve using the gui picker. The keyboard popping up every time you adjust a date/time is a very frustrating user experience. If we were able to come up with a compromise where by default on mobile it doesn't show the keyboard unless you manually chose to I think that would be ideal but as you mentioned I'm not sure if that's possible with the current library.

I'll give gulp updatestatic a try, thanks!

@cdubz cdubz temporarily deployed to babybuddy-pr-304 September 8, 2021 04:29 Inactive
@cdubz
Copy link
Member

cdubz commented Sep 8, 2021

@ntrecina could you open a new pr with just the breast milk autoselect change? That seems fine to me but I'd like to invite more discussion about the inputmode change.

@ntrecina
Copy link
Contributor Author

ntrecina commented Sep 9, 2021

@cdubz Sure thing.

@ntrecina ntrecina changed the title Feeding form and date picker changes Date picker changes Sep 9, 2021
@ntrecina
Copy link
Contributor Author

@Expro Do you have any thoughts on this change? From reading your comments on #284 it sounds like you would like to modify date/times via the keyboard on desktop. Would you also like that ability on mobile or do you think gui only control on mobile would be ok?

@Expro
Copy link

Expro commented Sep 16, 2021

I'm using BabyBuddy almost exclusively on mobile devices, so all my comments are valid for mobile. I still think separate button for GUI picker invocation that is placed adjacent to textboxt containing date and time is superior solution.

@cdubz
Copy link
Member

cdubz commented Sep 16, 2021

@Expro I see -- you are advocating for not showing the picker when clicking in the field but only when clicking the little calendar icon next to the field, right? I'm not sure to what extent that behavior is configurable with the library but that approach does seem like a good middle ground if we can sort it out.

@ntrecina what do you think?

@Expro
Copy link

Expro commented Sep 16, 2021

That right - I think editable field AND ability to launch picker by button / calendar icon covers both use cases intuitively and would be great if possible (as You have mentioned).

@ntrecina
Copy link
Contributor Author

That seems like a good compromise to satisfy both use cases. That being said I would still prefer an option (user preference maybe) to block keyboard input on mobile since my wife and I don't use it and it's nice having a large target to tap on rather than needing to tap on the calendar to avoid the keyboard opening.

@cdubz
Copy link
Member

cdubz commented Sep 17, 2021

Fair enough, thanks for contributing to the discussion @ntrecina and @Expro. I'm going to merge this change and we'll see how it goes. I think I need to put some more serious thought in to settings organization and options so that I am less overall adverse to adding toggles like this.

@cdubz cdubz merged commit a844aa7 into babybuddy:master Sep 17, 2021
@cdubz cdubz added enhancement Feature requests or improvements to existing functionality ui/ux Issues relating to user interface/experience labels Sep 17, 2021
@cdubz cdubz added this to the v1.9.0 milestone Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests or improvements to existing functionality ui/ux Issues relating to user interface/experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants