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

Add ability to select a range of dates #25

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

Add ability to select a range of dates #25

wants to merge 8 commits into from

Conversation

mattgrayson
Copy link

Allows setting the selection mode of the calendar view to either allow selecting a single date or a range of dates. Delegate methods have been added for start and end date selection/dedeselection. Had already started working on this before I saw @kemeran's and @randomstep's pull requests, so their changes (ability to deselect a date and improved scrolling method) are also included.

Range selection mimics Hipmunk. Also streamlines button creation.
support for deselecting dates and ensure calendarView:shouldSelectDate:
is respected when selecting date ranges
Was only scrolling to the month of the given date. Also added secondary
method to allow customizing the scroll position.
…ke into account self.pinsHeaderToTop

Also added date range selection demo to test app
@kemenaran
Copy link

I had the same need (selecting a date range), and ended up implementing a solution I wasn't quite satisfied with. This feature would probably be a worthy addition to TimesSquare.

Added public properties for text shadow color, today's text color,
today's shadow color. Also added public property to toggle display of
dates that fall outside of the current month.
The copy headers build phase wasn't making the headers available to the
parent project (and breaking the ability to Archive the parent
project). Moving headers to copy files build phase resolved the
problem. Not sure this is the correct fix ...
blakewatters pushed a commit to GateGuru/objc-TimesSquare that referenced this pull request Aug 7, 2013
@blakewatters
Copy link

@puls are there any specific design concerns or changes desired on this pull request? I am building out a calendar UI that requires mutli-date selection behaviors and would love to build against something that is destined for merge. I am happy to do any legwork to bring this PR up to code for a merge.

@puls
Copy link
Owner

puls commented Aug 7, 2013

Needs a merge against master.

@blakewatters
Copy link

@puls see #33

@puls
Copy link
Owner

puls commented Aug 7, 2013

Also needs a signed CLA from @mattgrayson.

@mattgrayson
Copy link
Author

CLA submitted

@blakewatters
Copy link

Couple of questions/comments on this:

  1. Why do we need a selectedDate as well as a startDate and endDate. Couldn't we just flatten out the distinction such that endDate is always nil when you are in single date mode?
  2. Why do we need didDeselect delegate methods? It seems like didSelectDate: with a nil date argument is sufficient.
  3. I think that the selection configuration is better described as a 'mode' rather than a 'type':
typedef enum {
    TSQCalendarSelectionModeDay = 0,
    TSQCalendarSelectionModeDateRange = 1
} TSQCalendarSelectionMode;

Couple of other changes coming up on #33

@mattgrayson
Copy link
Author

Been a while since I looked at this, so my memory might be off:

  1. I think adding startDate/endDate in addition to selectedDate was to preserve compatibility with the existing API.
  2. Makes sense
  3. Agree

Thanks for bringing up to date + fixes.

@blakewatters
Copy link

Updates pushed to #33 for items 2 and 3.

@puls Do you care about backward compatibility for the selectedDate property at the expense of the additional properties/delegate methods?

@puls
Copy link
Owner

puls commented Aug 12, 2013

I don't care about backward compatibility so much as simplicity; it seems to me that being able to set a single selected date with a single method is a pretty common use case.

(For that matter, it seems like having a selection mode is pretty overkill, too; you should be able to make -setSelectedDate: just call both -setSelectedStartDate: and -setSelectedEndDate: with its argument.)

@blakewatters
Copy link

The selection mode determines the behavior taken when you touch on the calendar, i.e. touching another date when one is already selected when in day selection mode moves the date, but in date range selection mode it creates a range between the two dates.

Maybe I am missing something, but I don't see how you can get rid of that without moving the calendar to exclusively supporting ranged selections. For my use-case, I have a calendar for selecting flight departure/return dates. If you are searching for One Way flights you only want a single date selection, but for Round Trip you want a range. The user can toggle between these modes, which is currently modeled nicely by the selection mode.

@puls
Copy link
Owner

puls commented Aug 12, 2013

Maybe we should punt on deciding what subsequent taps do; I can imagine a case where the first and second taps are relatively straightforward, but later ones are confusing.

Maybe a delegate callback: if the delegate doesn't respond, always select a single date; otherwise, do what the delegate says?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants