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.
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
docs: date picker spec #388
docs: date picker spec #388
Changes from all commits
830c08a
ca6e99c
4c3c6ff
2257146
09588c3
00a76e2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised that this doesn't support a parameter combination like
ui.date_picker(table: Table, date_col: str)
. I would expect to be able to pass a table in along with a column name for a column containing one of the valid types, and have the picker automatically provide the allowable date ranges accordingly. Not sure what the right default would be, maybe the entire date range?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mofojed @dsmmcken thoughts?
I was thinking about a table source but thought it didn't make much sense due to differing resolutions. I suppose making the column an alternative to
min_value
ormax_value
is possible, but seems to be a lot of work for something that could be done through a combination of table operations anduse_cell_data
hooks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea @alexpeters1208 for that case, wouldn't a
ui.picker
make more sense instead when selecting a date from a table? As for a range, yea I thinkuse_cell_data
would work fine for that case... we should mock out an example though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://react-spectrum.adobe.com/react-spectrum/DateRangePicker.html is a different thing, but also maybe relevant to the discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mofojed My understanding is that
date_picker
gives you a kind of calendar pop-up, whilepicker
is a drop-down list. I don't think I'd want to use a drop down list for dates, seems like that list gets really long really quick. Or maybe I'm not understanding?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fairly contrived and simplified example but represents how you could derive a value from a table for a range already. It's not difficult to do the end range too but I wanted to show that there is lots of flexibility that might be desired that I think is best achieved outside of the
date_picker
itself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you've excluded it, but the isDateUnavailable prop feels kinda like something that could be a column.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the same thought but quickly ran into trouble when considering how exactly that would be implemented. It doesn't make much sense as a separate boolean column because you would have to check the boolean + date column.
This brings me to the possibility of having a table for
unavailable_values
with a date column (which I basically emulate with a list of dates at the moment) but is it worth implementing table backing for that?