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

OGM-529 Can select whole month from a button in the Calendar #1323

Merged
merged 53 commits into from
Jun 22, 2022

Conversation

tr3sG
Copy link
Contributor

@tr3sG tr3sG commented Jun 7, 2022

We want to implement Whole Month searches, for a given origin/destination pair.
In order to allow users to perform flights searches for a whole month, we need a "Search whole month" button to the date selection header UI.
image

NOTE: There is still some work to do related to the selection highlighting. That will be covered in another PR (since that work belongs to another ticket OGM-530).

Remember to include the following changes:

If you are curious about how we review, please read through the code review guidelines

Copy link
Contributor

@frugoman frugoman left a comment

Choose a reason for hiding this comment

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

Please add the change on the UNRELEASED.md file

@frugoman frugoman added the major Breaking change label Jun 14, 2022
@frugoman
Copy link
Contributor

This may need rebasing

@tr3sG tr3sG changed the title OGM-529 Add whole month button to the Calendar OGM-529 Can select whole month from a button in the Calendar Jun 20, 2022
@tr3sG tr3sG force-pushed the OGM-529_add-whole-month-button branch from 48c6b66 to 2130e77 Compare June 20, 2022 10:52
@frugoman frugoman closed this Jun 20, 2022
@frugoman frugoman reopened this Jun 20, 2022
Backpack/Calendar/Classes/BPKCalendar.h Outdated Show resolved Hide resolved
Backpack/Calendar/Classes/BPKCalendar.m Show resolved Hide resolved
Backpack/Calendar/Classes/BPKCalendar.m Show resolved Hide resolved
Backpack/Calendar/Classes/BPKCalendar.m Outdated Show resolved Hide resolved
Comment on lines +439 to +440
BOOL previousMultiSelectionConfiguration = self.calendarView.allowsMultipleSelection;
self.calendarView.allowsMultipleSelection = YES;
Copy link
Contributor

Choose a reason for hiding this comment

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

What will allowsMultipleSelection do when we set it to YES in this function? It feels like a bit of a 'hack' considering we restore it at the end of the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've just realised I marked as resolved this question when it wasn't 🤦‍♂️
Yes, this is a hack, but it's the only way I could find to mark the whole month as selected when the selection configuration is simple (only one date can be selected), without this hack the only date marked as selected in single selection configuration is the last day of the month.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me, should we add a comment indicating that 'selecting whole month' does not take the calendar type into account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I just saw this comment right now, after is merged 😵‍💫
But yes, I think it makes sense, too.

Backpack/Calendar/Classes/BPKCalendarStickyHeader.m Outdated Show resolved Hide resolved
Backpack/Calendar/Classes/BPKCalendarStickyHeader.m Outdated Show resolved Hide resolved
Gerardo Garrido and others added 22 commits June 22, 2022 16:44
…hat highlighting logic under `wholeMonthSelection`.
…, last day and number of days of a given month.
@frugoman frugoman force-pushed the OGM-529_add-whole-month-button branch from 62130ca to 71c4f08 Compare June 22, 2022 15:49
@frugoman frugoman enabled auto-merge (squash) June 22, 2022 15:51
@frugoman frugoman merged commit aadf5d3 into main Jun 22, 2022
@frugoman frugoman deleted the OGM-529_add-whole-month-button branch June 22, 2022 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants