-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Respect showOutsideDays prop for single month date range #6862
base: develop
Are you sure you want to change the base?
Changes from all commits
0a156c1
4ec0b97
5ef0a98
2ed584c
8b74c8b
5a2b3a7
f55b73c
e4e88e5
62b901c
93f551b
592f938
d7f7021
f412b17
67d95bc
9856095
2cfa0c0
51512a9
e894b59
018fae3
c4a82a8
b8f0ec3
1121b81
aec0e5d
fd37b66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/* | ||
* Copyright 2024 Palantir Technologies, Inc. All rights reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
export const WARNING_IGNORED_SHOW_OUTSIDE_DAYS_PROP = | ||
"dayPickerProps.showOutsideDays will only be applied if the DateRangePicker3 is displaying a single month, either through the singleMonthOnly prop, or with a minDate and maxDate of the same month."; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,14 @@ | |
} | ||
} | ||
|
||
|
||
/* stylelint-disable max-line-length */ | ||
&.#{$ns}-datepicker.#{$ns}-daterangepicker-single-month.#{$ns}-daterangepicker-showing-outside-days .rdp-day_outside { | ||
pointer-events: none; // weird interactions happen if allowing outside days to be clicked | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that without this, selecting a date in the earlier "outside days" switches to the previous month. This doesn't seem ideal since there will not be many days ahead of that initial date selected once we switch to the previous month, but the user can reselect the month after making the start date selection. Also note that we only need to worry about click and not keyboard nav events here because we still want to allow keyboard nav with arrow keys to navigate between months. The difference is that keyboard nav is navigation, then selection, while mouse interaction is selection and navigation which felt a bit jarring. Open to suggestions for a better solution here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be curious for @alex-dales's opinion. Playing around with the UI, I actually found it surprising that I couldn't click on the grayed out dates. It might not be obvious to some users that they have to switch to the previous month intentionally to select these "outside day" dates? |
||
visibility: visible; // showing outside days is allowed for single months | ||
} | ||
/* stylelint-enable max-line-length */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: It should be possible to remove this after using |
||
|
||
// need some extra specificity to override DatePicker styles | ||
&.#{$ns}-datepicker .rdp-day { | ||
// we only want outside days to be shown when displaying one month at a time | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ import enUSLocale from "date-fns/locale/en-US"; | |
import { mount, type ReactWrapper } from "enzyme"; | ||
import * as React from "react"; | ||
import { type DayModifiers, DayPicker, type ModifiersClassNames } from "react-day-picker"; | ||
import sinon from "sinon"; | ||
import sinon, { stub } from "sinon"; | ||
|
||
import { Button, Classes, Menu, MenuItem } from "@blueprintjs/core"; | ||
import { | ||
|
@@ -1332,6 +1332,55 @@ describe("<DateRangePicker3>", () => { | |
}); | ||
}); | ||
|
||
describe("outside days", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these tests are very closely tied to the implementation but figured it was good to get something in testing the behavior so this doesn't unintentionally regress |
||
it("visually hides outside days when contiguous months are shown, even if showOutsideDays is true", () => { | ||
const warnSpy = stub(console, "warn"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: Might be useful to clean up this stub at the end of this function with: warnSpy.restore(); Otherwise this |
||
const { left } = render({ | ||
dayPickerProps: { showOutsideDays: true }, | ||
initialMonth: new Date(2015, Months.DECEMBER, 2), | ||
}); | ||
assert.strictEqual(warnSpy.callCount, 1); | ||
assert.equal( | ||
left.findOutsideDays().first().getDOMNode().computedStyleMap().get("visibility")?.toString(), | ||
"hidden", | ||
); | ||
}); | ||
|
||
it("allows outside days to be shown for single month date range", () => { | ||
const { left } = render({ | ||
dayPickerProps: { showOutsideDays: true }, | ||
initialMonth: new Date(2015, Months.DECEMBER, 2), | ||
singleMonthOnly: true, | ||
}); | ||
assert.equal( | ||
left.findOutsideDays().first().getDOMNode().computedStyleMap().get("visibility")?.toString(), | ||
"visible", | ||
); | ||
}); | ||
|
||
it("does not allow outside days to be selected even when visible", () => { | ||
const { left } = render({ | ||
dayPickerProps: { showOutsideDays: true }, | ||
initialMonth: new Date(2015, Months.DECEMBER, 2), | ||
singleMonthOnly: true, | ||
}); | ||
|
||
assert.equal( | ||
left.findOutsideDays().first().getDOMNode().computedStyleMap().get("pointer-events")?.toString(), | ||
"none", | ||
); | ||
|
||
// simulated clicks appear to not respect pointerEvents style or else would test with below | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no activity on it but I found enzymejs/enzyme#2340 open to suggestions for better ways to disable the interaction or better ways to test this |
||
// | ||
// November 29th is first outside day shown for December 2015 | ||
// assert.equal(left.findOutsideDays().first().text(), "29"); | ||
// | ||
// this click would navigate to November if interaction was allowed | ||
// left.findOutsideDays().first().simulate("click"); | ||
// left.assertDisplayMonth(Months.DECEMBER, 2015); | ||
}); | ||
}); | ||
|
||
function dayNotOutside(day: ReactWrapper) { | ||
return !day.hasClass(Datetime2Classes.DATEPICKER3_DAY_OUTSIDE); | ||
} | ||
|
@@ -1460,10 +1509,12 @@ describe("<DateRangePicker3>", () => { | |
return harness | ||
.findDays() | ||
.filterWhere(day => day.text() === "" + dayNumber) | ||
.filterWhere(day => !day.hasClass(Datetime2Classes.DATEPICKER3_DAY_OUTSIDE)) | ||
.filterWhere(dayNotOutside) | ||
.first(); | ||
}, | ||
findDays: () => harness.wrapper.find(`.${Datetime2Classes.DATEPICKER3_DAY}`), | ||
findOutsideDays: () => | ||
harness.wrapper.find(`.${Datetime2Classes.DATEPICKER3_DAY}`).filterWhere(day => !dayNotOutside(day)), | ||
mouseEnterDay: (dayNumber = 1) => { | ||
harness.findDay(dayNumber).simulate("mouseenter"); | ||
return harness; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,8 @@ | |
|
||
import * as React from "react"; | ||
|
||
import { Classes, FormGroup, Switch } from "@blueprintjs/core"; | ||
import type { DateRange, TimePrecision } from "@blueprintjs/datetime"; | ||
import { Classes, FormGroup, Switch, Tooltip } from "@blueprintjs/core"; | ||
import { type DateRange, DateUtils, type TimePrecision } from "@blueprintjs/datetime"; | ||
import { DateRangePicker3 } from "@blueprintjs/datetime2"; | ||
import { Example, type ExampleProps, handleBooleanChange, handleValueChange } from "@blueprintjs/docs-theme"; | ||
|
||
|
@@ -41,6 +41,7 @@ interface DateRangePicker3ExampleState { | |
showTimeArrowButtons: boolean; | ||
timePrecision: TimePrecision | undefined; | ||
useAmPm: boolean; | ||
showOutsideDays: boolean; | ||
} | ||
|
||
export class DateRangePicker3Example extends React.PureComponent<ExampleProps, DateRangePicker3ExampleState> { | ||
|
@@ -53,6 +54,7 @@ export class DateRangePicker3Example extends React.PureComponent<ExampleProps, D | |
minDate: undefined, | ||
reverseMonthAndYearMenus: false, | ||
shortcuts: true, | ||
showOutsideDays: true, | ||
showTimeArrowButtons: false, | ||
singleMonthOnly: false, | ||
timePrecision: undefined, | ||
|
@@ -91,6 +93,8 @@ export class DateRangePicker3Example extends React.PureComponent<ExampleProps, D | |
this.setState({ contiguousCalendarMonths }); | ||
}); | ||
|
||
private toggleShowOutsideDays = handleBooleanChange(showOutsideDays => this.setState({ showOutsideDays })); | ||
|
||
public render() { | ||
const { dateRange, localeCode, maxDate, minDate, showTimeArrowButtons, timePrecision, useAmPm, ...props } = | ||
this.state; | ||
|
@@ -105,6 +109,9 @@ export class DateRangePicker3Example extends React.PureComponent<ExampleProps, D | |
maxDate={maxDate} | ||
minDate={minDate} | ||
onChange={this.handleDateRangeChange} | ||
dayPickerProps={{ | ||
showOutsideDays: this.state.showOutsideDays, | ||
}} | ||
timePickerProps={ | ||
showTimePicker | ||
? { precision: timePrecision, showArrowButtons: showTimeArrowButtons, useAmPm } | ||
|
@@ -131,6 +138,18 @@ export class DateRangePicker3Example extends React.PureComponent<ExampleProps, D | |
label="Single month only" | ||
onChange={this.toggleSingleMonth} | ||
/> | ||
<Tooltip content="Only respected when showing a single month calendar"> | ||
<Switch | ||
checked={this.state.showOutsideDays} | ||
className="keep-bottom-margin" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
disabled={ | ||
!this.state.singleMonthOnly && | ||
!DateUtils.isSameMonth(this.state.minDate, this.state.maxDate) | ||
} | ||
label="Show outside days" | ||
onChange={this.toggleShowOutsideDays} | ||
/> | ||
</Tooltip> | ||
<Switch | ||
checked={this.state.contiguousCalendarMonths} | ||
disabled={this.state.singleMonthOnly} | ||
|
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.
nitpick: I usually like to use
stylelint-disable-next-line
whenever possible to avoid the need to re-enable the lint rule further down.