Skip to content
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

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/datetime/src/common/classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const DATERANGEPICKER = `${NS}-daterangepicker`;
export const DATERANGEPICKER_CALENDARS = `${DATERANGEPICKER}-calendars`;
export const DATERANGEPICKER_CONTIGUOUS = `${DATERANGEPICKER}-contiguous`;
export const DATERANGEPICKER_SINGLE_MONTH = `${DATERANGEPICKER}-single-month`;
export const DATERANGEPICKER_SHOWING_OUTSIDE_DAYS = `${DATERANGEPICKER}-showing-outside-days`;
export const DATERANGEPICKER_DAY_SELECTED_RANGE = `${DATEPICKER_DAY}--selected-range`;
export const DATERANGEPICKER_DAY_HOVERED_RANGE = `${DATEPICKER_DAY}--hovered-range`;
export const DATERANGEPICKER_SHORTCUTS = `${DATERANGEPICKER}-shortcuts`;
Expand Down
18 changes: 18 additions & 0 deletions packages/datetime2/src/common/errors.ts
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
Expand Up @@ -59,6 +59,14 @@
}
}


/* stylelint-disable max-line-length */
Copy link
Contributor

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.

Suggested change
/* stylelint-disable max-line-length */
// stylelint-disable-next-line 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: It should be possible to remove this after using // stylelint-disable-next-line above.


// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ The **preset shortcuts** can be seen in the example above. They are as follows:

@## Props interface

The `showOutsideDays` prop on `dayPickerProps` will only be respected when a single month is shown.
This is to avoid a possible double representation of a selected date range.

@interface DateRangePicker3Props

@## Localization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {

import { Classes, dayPickerClassNameOverrides } from "../../classes";
import { combineModifiers, HOVERED_RANGE_MODIFIER } from "../../common/dayPickerModifiers";
import { WARNING_IGNORED_SHOW_OUTSIDE_DAYS_PROP } from "../../common/errors";
import { DatePicker3Provider } from "../date-picker3/datePicker3Context";
import { DateFnsLocalizedComponent } from "../dateFnsLocalizedComponent";

Expand Down Expand Up @@ -120,6 +121,10 @@ export class DateRangePicker3 extends DateFnsLocalizedComponent<DateRangePicker3
time,
value,
};

if (!getIsSingleMonthOnly(this.props) && this.props.dayPickerProps?.showOutsideDays) {
console.warn(WARNING_IGNORED_SHOW_OUTSIDE_DAYS_PROP);
}
}

public render() {
Expand All @@ -131,6 +136,7 @@ export class DateRangePicker3 extends DateFnsLocalizedComponent<DateRangePicker3
[Classes.DATERANGEPICKER_CONTIGUOUS]: contiguousCalendarMonths,
[Classes.DATERANGEPICKER_SINGLE_MONTH]: isSingleMonthOnly,
[Classes.DATERANGEPICKER3_REVERSE_MONTH_AND_YEAR]: this.props.reverseMonthAndYearMenus,
[Classes.DATERANGEPICKER_SHOWING_OUTSIDE_DAYS]: this.props.dayPickerProps?.showOutsideDays,
});

// use the left DayPicker when we only need one
Expand Down
55 changes: 53 additions & 2 deletions packages/datetime2/test/components/dateRangePicker3Tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1332,6 +1332,55 @@ describe("<DateRangePicker3>", () => {
});
});

describe("outside days", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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");
Copy link
Contributor

Choose a reason for hiding this comment

The 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 console.warn stub stucks around for all tests that run after this one.

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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);
}
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -41,6 +41,7 @@ interface DateRangePicker3ExampleState {
showTimeArrowButtons: boolean;
timePrecision: TimePrecision | undefined;
useAmPm: boolean;
showOutsideDays: boolean;
}

export class DateRangePicker3Example extends React.PureComponent<ExampleProps, DateRangePicker3ExampleState> {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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 }
Expand All @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nitpick: It looks like the bottom margin here is getting reset back to 0 by some other CSS selector.

Screenshot 2024-08-06 at 6 08 13 PM

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}
Expand Down
7 changes: 4 additions & 3 deletions packages/docs-theme/src/styles/_examples.scss
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ $example-spacing: $pt-grid-size * 4;
font-size: $pt-font-size-small;
}

.#{$ns}-control:last-child,
.#{$ns}-form-group:last-child,
.#{$ns}-label:last-child {
// add a "keep-bottom-margin" class to avoid removing the bottom margin due to the tooltip wrapper
.#{$ns}-control:last-child:not(.keep-bottom-margin),
.#{$ns}-form-group:last-child:not(.keep-bottom-margin),
.#{$ns}-label:last-child:not(.keep-bottom-margin) {
margin-bottom: 0;
}

Expand Down