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
Update to use dmc 0.15.1 #924
base: main
Are you sure you want to change the base?
Update to use dmc 0.15.1 #924
Changes from 4 commits
1a962f1
7ff1b73
efbd801
7e1ff1c
4a091d2
fd2aeff
4189fa8
cfb1c3b
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.
Good point! The
range
argument is definitely a bit redundant if we ever want to introduce thetype
argument of the newdmc.DatePickerInput
as well. Options would then include: "default", "range", "multiple" in which case this boolean argument doesn't make much sense as you could then also define it via thetype
directly, but probably better to keep it for backwards compatibility even though it might be double-defined then.Let's leave it in for now and discuss when @antonymilne is back 👍
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.
Antony previously mentioned the concept of a "multiple" option for the DatePicker in #318 (comment).
At this point, I believe we don't need to support the "multiple" mode yet, as it represents a niche use case. If we decide to implement this feature, we should also consider extending similar functionality to other components, such as vm.Slider. This would align with broader discussions about restructuring our components (See -> #318 (comment))).
For now, the
range
property seems to address the requirements effectively.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.
Ah, perfect! I didn't know he commented on this already. Let's leave it as is then, and @AnnMarieW you could delete this line then:
date_range_picker_kwargs = {"allowSingleDateInRange": True} if self.range else {}
I don't think it's needed anymore since that is controlled by the
type
argument now instead ofallowSingleDateInRange
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.
We should always include
allowSingleDateInRange=True,
in thedmc.DatePickerInput
configuration. It will enable that a single date can be selected whenrange=True
(sotype="range"
), and will have no effect whenrange=False
(sotype="default"
).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.
Ah I see, but then let's specify it directly as
allowSingleDateInRange=True
instead of adding that line 👍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'm a little confused by this - not sure what props to update 🤔
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.
All that is needed is to delete the following line:
date_range_picker_kwargs = {"allowSingleDateInRange": True} if self.range else {}
and insert
allowSingleDateInRange: True
directly to thedmc.DatePickerInput
configuration like in this suggestion comment.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.
Saw that all of these issues have been fixed in the mantine repository @AnnMarieW - this is amazing! 🥳
@nadijagraca @petar-qb - could you double-check manually if all of the issues we've faced initially are solved now? I remember for one of the bugs, we had to add the datepicker to the right side and then change screen sizes etc. I don't know if there were other scenarios like that which need manual checking. You will know best 👍
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.
After some testing with various configuration inputs, I can say that all our previous workaround solutions are unnecessary as
dmc.DatePickerInput
handles them all internally 🎉 Here's the example of our workaround solutions.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 suggestion is only used as a reference from another comment.