[SHIBA-2119] Added Optional Height Prop for Scrollable Calendar #3057
Conversation
| @@ -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; | |||
There was a problem hiding this comment.
Moved this down, so it can be assigned by the optional prop.
There was a problem hiding this comment.
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
There was a problem hiding this 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.
| @@ -66,6 +66,7 @@ export type Props = { | |||
| initiallyFocusedDate?: Date | null; | |||
| markToday?: boolean; | |||
| markOutsideDays?: boolean; | |||
| monthHeight?: number; | |||
There was a problem hiding this comment.
We don't need to specify this prop specifically as the <Calendar> component passes the rest of the props via {...calendarProps}
See here
|
Visit https://backpack.github.io/storybook-prs/3057 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3057 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3057 to see this build running in a browser. |
| // 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 = monthHeight; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
That makes sense.
I've updated it so that we're passing a prop for ROW_HEIGHT.
|
Visit https://backpack.github.io/storybook-prs/3057 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3057 to see this build running in a browser. |
AsimAM
changed the title
|
Visit https://backpack.github.io/storybook-prs/3057 to see this build running in a browser. |
| @@ -60,11 +55,13 @@ type Props = Partial<BpkCalendarGridProps> & { | |||
| focusedDate?: Date | null; | |||
| selectionConfiguration?: SelectionConfiguration; | |||
| className?: string | null; | |||
| customRowHeight?: number; | |||
There was a problem hiding this comment.
Might be worth adding a JSDOC comment to specify this value should be in rem - and same with the other components where this is part of the API. That's gonna reflect in the Storybook API docs then too automatically!
There was a problem hiding this comment.
Thanks - I opted for regular comments instead of JSDOC comments due to consistency (a lot of the other props are being specified by regular comments) and it would look odd to have JSDoc for a singular prop. But happy to change it if it is still the preference.
|
Visit https://backpack.github.io/storybook-prs/3057 to see this build running in a browser. |
We want to be able to pass an optional heigh prop to add extra whitespace between months.
See the images below for reference.
Remember to include the following changes:
README.md(If you have created a new component)README.md