search

trynmaps / metrics-mvp

Prototype of public transit data visualization system
https://muni.opentransit.city
MIT License
30 stars 34 forks source link

detects when none of the days of week selected are in the date range … #628

Closed Brian-Lee closed 4 years ago

Brian-Lee commented 4 years ago

Proposed changes

This detects when the user has not selected any days of the week that are actually included in their date range, alerts them with a snackbar, and disables the 'Apply' button.

Without this functionality, the user would see an ugly error msg about arrays to concatenate. ...

Demo

https://metrics-mvp-brians-branch.herokuapp.com/

Screenshot

image

Brian-Lee commented 4 years ago

The first commit has the functionality. The rest of the commits deal with lint errors. Many of those lint errors were in the file I changed (DateTimePopover.jsx), but I also had to fix lint errors in many files that I didn't originally change for some reason. I suspect this is due to changing lint requirements.

hathix commented 4 years ago

Let me test your branch locally. One quick suggestion: I would use a Material Snackbar (https://material-ui.com/components/snackbars/) instead of a native JS alert. Using Material will feel less disruptive to the user, and more professional.

hathix commented 4 years ago

Also, what happens when you dismiss the alert? Does the user's change not get applied?

Now that I think about it, I think the best UX would be to disable the "apply" button (and not apply any changes) if the user has chosen the wrong set of days of the week (like when none of the days of the week are in the date range). This would prevent this error from happening in the first place.

hathix commented 4 years ago

OK, I tried it locally and it indeed lets you apply the change even after showing the alert, which leads to a screen with no data. Let's just disable the "apply" button when invalid information is entered in the dropdown, as I suggested in the last column.

Brian-Lee commented 4 years ago

Let me test your branch locally. One quick suggestion: I would use a Material Snackbar (https://material-ui.com/components/snackbars/) instead of a native JS alert. Using Material will feel less disruptive to the user, and more professional.

I managed to change the native javacript alert to a snackbar.

Brian-Lee commented 4 years ago

I have incorporated much of Neels great ideas/feedback. Please review.