Better describe UI factor options in readme #165
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AdK0101
reviewed
Dec 5, 2024
| | ----- | ----------- | | ||
| | Corridor | Drawn on the map, it is a shortest path between two intersections of your choice. Draw it in both directions if you need both directions of travel. | | ||
| | Time Range | Times must start and end on the hour and the app accepts integer values between 0 and 24. The final hour is _exclusive_, meaning that a range of 7am to 9am covers two hours, not three. Values of 0 and 24 both interchangeably represent midnight; a time range of 0 - 24 will return all hours of the day. A time range starting after it ends (e.g. 10pm to 4am) will wrap around midnight. | | ||
| | Date Range | Use the calendar widget to select a date range. Note that selected ranges are displayed with an exclusive end date. | |
There was a problem hiding this comment.
Is it worth discussing what happens when you wrap around midnight? If you have a range of [day, day+1), the next day is not included. What happens when you input for your time, say [22, 5)? This may be confusing for the reader.
| | Corridor | Drawn on the map, it is a shortest path between two intersections of your choice. Draw it in both directions if you need both directions of travel. | | ||
| | Time Range | Times must start and end on the hour and the app accepts integer values between 0 and 24. The final hour is _exclusive_, meaning that a range of 7am to 9am covers two hours, not three. Values of 0 and 24 both interchangeably represent midnight; a time range of 0 - 24 will return all hours of the day. A time range starting after it ends (e.g. 10pm to 4am) will wrap around midnight. | | ||
| | Date Range | Use the calendar widget to select a date range. Note that selected ranges are displayed with an exclusive end date. | | ||
| | Day of Week | Identify the days of week to include in the aggregation. | |
There was a problem hiding this comment.
Same kind of thing as the day. If you only want Monday data, but you wrap around to Tuesday in your time range, what happens? May be worth just discussing the implications of the wrap around thing somewhere in the readme.
|
Good thoughts. I'll try to clarify that. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #164
I started expanding the description of the time ranges and realized I'd need to change the list to a table to keep things readable. Perhaps still not perfectly clear, but I think this is better than the quick list that was there before.
@AdK0101 if you could give this a quick skim just to make sure it mostly makes sense to you too.