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.

Sign up for GitHub

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

Jump to bottom

[SHIBA-2119] Added Optional Height Prop for Scrollable Calendar #3057

Merged
merged 10 commits into from
Nov 7, 2023
Merged

[SHIBA-2119] Added Optional Height Prop for Scrollable Calendar #3057

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
eb1466a
added month height prop
Nov 2, 2023
4550052
oops
Nov 2, 2023
6b7c29e
update dec files
Nov 2, 2023
f0ffeb2
Empty-Commit
Nov 2, 2023
fa77fb8
replace monthHeight with rowHeight
Nov 6, 2023
b48ac34
customRowHeight
Nov 6, 2023
38b97de
rename consts
Nov 6, 2023
581fa96
rename consts
Nov 6, 2023
492c58c
added comments for customRowHeight prop
Nov 7, 2023
8cec8cd
fix comment
Nov 7, 2023
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
19 changes: 19 additions & 0 deletions examples/bpk-component-scrollable-calendar/examples.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,24 @@ const DefaultExample = () => (
/>
);

const DefaultExampleWithCustomHeight = () => (
<ScrollableCal
weekStartsOn={1}
daysOfWeek={weekDays}
formatMonth={formatMonth}
formatDateFull={formatDateFull}
DateComponent={BpkScrollableCalendarDate}
minDate={new Date(2020, 3, 1)}
maxDate={new Date(2020, 6, 1)}
selectionConfiguration={{
type: CALENDAR_SELECTION_TYPE.single,
date: new Date(2020, 3, 15),
}}
monthHeight={10}
/>
);


const RangeExample = () => (
<ScrollableCal
weekStartsOn={1}
Expand Down Expand Up @@ -349,6 +367,7 @@ const PastCalendarExample = () => (

export {
DefaultExample,
DefaultExampleWithCustomHeight,
RangeExample,
SplitWeekRangeExample,
WeekStartsOnSixExample,
Expand Down
7 changes: 7 additions & 0 deletions examples/bpk-component-scrollable-calendar/stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
PastCalendarExample,
RangeExample,
SplitWeekRangeExample,
DefaultExampleWithCustomHeight,
} from './examples';

export default {
Expand Down Expand Up @@ -92,6 +93,12 @@ VisualTestWithZoom.args = {
zoomEnabled: true
};

export const VisualTestWithCustomHeight = DefaultExampleWithCustomHeight;
export const VisualTestWithCustomHeightWithZoom = VisualTestWithCustomHeight.bind({});
VisualTestWithCustomHeightWithZoom.args = {
zoomEnabled: true
};

export const VisualTestRange = RangeExample;
export const VisualTestRangeWithZoom = VisualTestRange.bind({});
VisualTestRangeWithZoom.args = {
Expand Down
1 change: 1 addition & 0 deletions packages/bpk-component-calendar/src/BpkCalendarContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export type Props = {
initiallyFocusedDate?: Date | null;
markToday?: boolean;
markOutsideDays?: boolean;
monthHeight?: number;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to specify this prop specifically as the <Calendar> component passes the rest of the props via {...calendarProps}

See here

};

type InjectedProps = {
Expand Down
3 changes: 3 additions & 0 deletions packages/bpk-component-calendar/src/composeCalendar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export type Props = {
preventKeyboardFocus?: boolean;
selectionConfiguration?: SelectionConfiguration;
gridClassName?: string | null;
monthHeight?: number;
/**
* Key to be used to pick the desired weekDay format from the `daysOfWeek` object, for example: `nameAbbr` or `nameNarrow`.
*/
Expand Down Expand Up @@ -132,6 +133,7 @@ const composeCalendar = (
maxDate,
minDate,
month,
monthHeight,
navProps = {},
nextMonthLabel = null,
onDateClick = null,
Expand Down Expand Up @@ -216,6 +218,7 @@ const composeCalendar = (
className={gridClasses.join(' ')}
dateProps={dateProps}
selectionConfiguration={selectionConfiguration}
monthHeight={monthHeight}
{...gridProps}
/>
</div>
Expand Down
16 changes: 16 additions & 0 deletions packages/bpk-component-scrollable-calendar/src/BpkScrollableCalendarGridList-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,22 @@ describe('BpkCalendarScrollGridList', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with a custom "monthHeight" attribute', () => {
const { asFragment } = render(
<BpkScrollableCalendarGridList
minDate={DateUtils.addDays(testDate, -1)}
maxDate={DateUtils.addMonths(testDate, 12)}
month={testDate}
formatMonth={formatMonth}
formatDateFull={formatDateFull}
DateComponent={BpkCalendarScrollDate}
weekStartsOn={0}
monthHeight={10}
/>,
);
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with a custom date component', () => {
const MyCustomDate = (props: any) => {
const cx = {
Expand Down
30 changes: 16 additions & 14 deletions packages/bpk-component-scrollable-calendar/src/BpkScrollableCalendarGridList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,6 @@ import { getMonthsArray, getMonthItemHeights } from './utils';

const getClassName = cssModules(STYLES);

// These constants are here to facilitate calculating the height
// Row and month item heights are defined in rem to support text scaling
const ROW_HEIGHT = 2.75;
// This is the additional height of each grid without any rows.
const BASE_MONTH_ITEM_HEIGHT = 8.125;
const COLUMN_COUNT = 7;
// Most browsers have by default 16px root font size
const DEFAULT_ROOT_FONT_SIZE = 16;
// Most calendar grids have 5 rows. Calculate height in px as this is what react-window expects.
const ESTIMATED_MONTH_ITEM_HEIGHT =
(BASE_MONTH_ITEM_HEIGHT + 5 * ROW_HEIGHT) * DEFAULT_ROOT_FONT_SIZE;
// Minimum month item width (useful for server-side rendering. This value will be overridden with an accurate width after mounting)
const ESTIMATED_MONTH_ITEM_WIDTH = BASE_MONTH_ITEM_HEIGHT * 7 * DEFAULT_ROOT_FONT_SIZE;

type Props = Partial<BpkCalendarGridProps> & {
minDate: Date;
maxDate: Date;
Expand All @@ -60,13 +46,15 @@ type Props = Partial<BpkCalendarGridProps> & {
focusedDate?: Date | null;
selectionConfiguration?: SelectionConfiguration;
className?: string | null;
monthHeight?: number;
};

const BpkScrollableCalendarGridList = (props: Props) => {
const {
className = null,
focusedDate = null,
minDate,
monthHeight = 8.125,
selectionConfiguration,
...rest
} = props;
Expand All @@ -75,6 +63,20 @@ const BpkScrollableCalendarGridList = (props: Props) => {
const endDate = startOfDay(startOfMonth(rest.maxDate));
const monthsCount = DateUtils.differenceInCalendarMonths(endDate, startDate);

// These constants are here to facilitate calculating the height
// Row and month item heights are defined in rem to support text scaling
const ROW_HEIGHT = 2.75;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this down, so it can be assigned by the optional prop.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we need to move any constants within the component and having this run with every render, we need to be careful what we move and also we shouldn't be using capital letters for these

Copy link
Contributor Author

@AsimAM AsimAM Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

#3057 (comment)

Yeah, makes sense.

With us now updating the ROW_HEIGHT via the prop, we don't need to move all of these constants down.

We will be in a middle state though where some dimension properties are still constant whilst others are not.

// This is the additional height of each grid without any rows.
const BASE_MONTH_ITEM_HEIGHT = monthHeight;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this constant actually represents the additional height of the grid month so the white spaces around and between rows. I think it might get confusing if we use the monthHeight prop for it.
I was thinking of having a prop for a row, rather than a month. But we could also do it like this, but would need to rename the prop as monthHeight sounds like it's a prop for the whole month which is a bit confusing as not each month will be always having the same size

Copy link
Contributor Author

@AsimAM AsimAM Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That makes sense.

I've updated it so that we're passing a prop for ROW_HEIGHT.

const COLUMN_COUNT = 7;
// Most browsers have by default 16px root font size
const DEFAULT_ROOT_FONT_SIZE = 16;
// Most calendar grids have 5 rows. Calculate height in px as this is what react-window expects.
const ESTIMATED_MONTH_ITEM_HEIGHT =
(BASE_MONTH_ITEM_HEIGHT + 5 * ROW_HEIGHT) * DEFAULT_ROOT_FONT_SIZE;
// Minimum month item width (useful for server-side rendering. This value will be overridden with an accurate width after mounting)
const ESTIMATED_MONTH_ITEM_WIDTH = BASE_MONTH_ITEM_HEIGHT * 7 * DEFAULT_ROOT_FONT_SIZE;

const getInitialRootFontSize = () =>
parseFloat(getComputedStyle(document.documentElement).fontSize) ||
DEFAULT_ROOT_FONT_SIZE;
Expand Down
Loading
Loading