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

dev/user-interface#47 Disable scrolling on time inputs #23264

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

mlutfy
Copy link
Member

@mlutfy mlutfy commented Apr 20, 2022

Overview

The time widgets used by CiviCRM are a bit sensitive. Combined to the fact that we often have rather long forms (ex: Event Management), an admin can easily accidentally change the time of an event.

To reproduce:

  • Go to Event > New Event
  • Scroll on the page
  • scroll over a time widget (ex: time of the event)

notice how the time was changed.

I feel like this is not useful at all, because the scrolling is super sensitive, and I find it pretty hard to really set the right time using scrolling.

https://lab.civicrm.org/dev/user-interface/-/issues/47

Before

ui47scrolling-fixed

After

Scrolling over the time input will scroll the page, but not the time.

Technical Details

Widget docs: http://keith-wood.name/timeEntry.html

Comments

A few users complained about this. The scrolling time makes them very uncomfortable about accidentally breaking things.

Drupal9 devs testing this need to run composer so that the /libraries folder is updated as well.

@civibot
Copy link

civibot bot commented Apr 20, 2022

(Standard links)

@civibot civibot bot added the master label Apr 20, 2022
@colemanw
Copy link
Member

I'm OK with this change. Maybe the reason it hasn't been raised before is because scroll-on-hover is a linux thing. On Mac/Win systems I'm pretty sure the input element would need to be focused before the scroll wheel would affect it.

@colemanw colemanw added the merge ready PR will be merged after a few days if there are no objections label Apr 20, 2022
@eileenmcnaughton
Copy link
Contributor

I had an internal ticket to fix this back when we upgraded to 4.4....

@mlutfy
Copy link
Member Author

mlutfy commented Apr 20, 2022

I had reports from users on Windows. Maybe the behaviour changed in recent Windows versions?

I'm not sure why the tests are failing. I see maybe stuff like this:


20 04 2022 06:28:41.728:INFO [karma-server]: Karma v6.3.17 server started at http://localhost:9876/
20 04 2022 06:28:41.730:INFO [launcher]: Launching browsers PhantomJS with concurrency unlimited
20 04 2022 06:28:41.734:INFO [launcher]: Starting browser PhantomJS
20 04 2022 06:28:42.071:INFO [PhantomJS 2.1.1 (Linux x86_64)]: Connected on socket jtlNlfsuj2h9XLeAAAAB with id 18734001
........................................................................
ALERT: 'Error
The scheduled date and time is in the past'
ALERT: 'Error
The scheduled date and time is in the past'
.
ALERT: 'Error
The scheduled date and time is in the past'
........
PhantomJS 2.1.1 (Linux x86_64): Executed 81 of 81 SUCCESS (1.04 secs / 0.993 secs)
Found EXITCODES=""

@mlutfy
Copy link
Member Author

mlutfy commented Apr 20, 2022

jenkins, test this please

@colemanw
Copy link
Member

@mlutfy all PR tests are failing at the moment. Might be an issue with the server?

@demeritcowboy
Copy link
Contributor

It's a windows thing too. It's also the cause of mysterious crashes when you go to edit a contribution and don't realize you've accidentally set a time for the receipt date but not a date part.

@totten
Copy link
Member

totten commented Apr 20, 2022

civibot, test this please

@eileenmcnaughton eileenmcnaughton merged commit 7b7c21c into civicrm:master Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants