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

Add disableMonthScroll #2524

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add disableMonthScroll #2524

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
59f2049
add disableMonthScroll
LeXuanKhanh Sep 8, 2024
cacd671
update document
LeXuanKhanh Sep 8, 2024
07ea7ad
fix wording
LeXuanKhanh Sep 18, 2024
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
5 changes: 5 additions & 0 deletions docsRNC/docs/Components/ExpandableCalendar.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ Whether to have shadow/elevation for the calendar
Whether to disable the week scroll in closed position
<span style={{color: 'grey'}}>boolean</span>

### disablMonthScroll

Whether to disable the month scroll in opened position
<span style={{color: 'grey'}}>boolean</span>

### openThreshold

The threshold for opening the calendar with the pan gesture
Expand Down
5 changes: 4 additions & 1 deletion src/expandableCalendar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ export interface ExpandableCalendarProps extends CalendarListProps {
allowShadow?: boolean;
/** whether to disable the week scroll in closed position */
disableWeekScroll?: boolean;
/** whether to disable the month scroll in opened position */
disableMonthScroll?: boolean;
/** a threshold for opening the calendar with the pan gesture */
openThreshold?: number;
/** a threshold for closing the calendar with the pan gesture */
Expand Down Expand Up @@ -106,6 +108,7 @@ const ExpandableCalendar = (props: ExpandableCalendarProps) => {
rightArrowImageSource = RIGHT_ARROW,
allowShadow = true,
disableWeekScroll,
disableMonthScroll,
openThreshold = PAN_GESTURE_THRESHOLD,
closeThreshold = PAN_GESTURE_THRESHOLD,
closeOnDayPress = true,
Expand Down Expand Up @@ -580,7 +583,7 @@ const ExpandableCalendar = (props: ExpandableCalendarProps) => {
onDayPress={_onDayPress}
onVisibleMonthsChange={onVisibleMonthsChange}
pagingEnabled
scrollEnabled={isOpen}
scrollEnabled={isOpen && !disableMonthScroll}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since CalendarList props allow passing ScrollView's scrollEnabled I prefer to use this one instead of adding another prop to the API.
Let's try moving line 586 before the {...others} (line 579) to allow overriding the 'scrollEnabled' prop by passing it instead of 'disableMonthScroll' to the ExpandableCalendar component.

Copy link
Author

Choose a reason for hiding this comment

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

Do you think it will confuse the user because we already had the disableWeekScroll prop ? They will think that using the prop scrollEnabled will disable scrolling behavior for both states opened and closed, not only opened. For that, I think we should use another prop for clarity.
If you insist on making the change in your opinion please let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand your point, but in the case of 'disableWeekScroll' we change the component from WeekCalendar to a simple Week component. In your implementation you're not replacing the CalendarList with a simple Calendar component (which is an option), but only passing it to the CalendarList's FlatList implementation.

Copy link
Author

Choose a reason for hiding this comment

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

I tried to do the same logic as the WeekCalendar but seems using the normal Calendar component it will break. So in this case must we use the scrollEnabled ?

Copy link
Author

Choose a reason for hiding this comment

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

any update @Inbal-Tish ?

hideArrows={shouldHideArrows}
onPressArrowLeft={_onPressArrowLeft}
onPressArrowRight={_onPressArrowRight}
Expand Down