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

Listener when tapping the background (e.g. for creating an event) #18

Closed
raLaaaa opened this issue Jun 17, 2020 · 6 comments · Fixed by #20
Closed

Listener when tapping the background (e.g. for creating an event) #18

raLaaaa opened this issue Jun 17, 2020 · 6 comments · Fixed by #20
Assignees
Labels
T: Feature Type: :tada: New Features

Comments

@raLaaaa
Copy link
Contributor

raLaaaa commented Jun 17, 2020

Hello there,

any ideas on when this will be live?
Are you open for foreign contributions?

I could not find a contribution guide but I'd be happy to start working on this one since I could need it for a personal project.

Just wanted to check in whether you are basically open for foreign contributions etc. and what a foreign contributor should consider?

Would be a pity if I start working on this one and it's nearly done on your side or you don't want anyone to contribute to this project.

@raLaaaa raLaaaa added the T: Feature Type: :tada: New Features label Jun 17, 2020
@JonasWanke
Copy link
Owner

Yes, I'm definitely open to contributions and haven't started implementing this feature yet. Just fork the project and open a PR when you're done or a draft PR when you have something to discuss. You don't have to update the changelog or version as I do that directly when releasing. Maybe showcase it in the example project by showing a snackbar when the listener fires.

I, unfortunately, don't have any documentation about the architecture of timetable yet (which will soon change a lot due to #17), so feel free to ask if you have any questions. There will probably be an additional parameter in the Timetable widget (like onCreateEvent) of type typedef OnCreateEventCallback = void Function(LocalDateTime start, bool isAllDay); that receives the tapped date and time (or the day at midnight when tapping the header). That listener then gets passed down to the DateEvents and AllDayEvents widgets where the raw touch events are received and converted.

@raLaaaa
Copy link
Contributor Author

raLaaaa commented Jun 18, 2020

I added the typedef to the Timetable. I see the point to pass that listener down to DateEvents and AllDayEvents. What would be the best way to pass that listener down?

Would we use the eventBuilder for that?

EG (for simple events).

        Expanded(
          child: TimetableContent<E>(
            controller: controller,
            eventBuilder: eventBuilder, <-- pass it as parameter or such?
          ),
        ),

Could you maybe shortly elaborate on the difference between date_events and the event? I see that date_events contain a list of events. I just want to make sure that I get it right to understand where we exactly have to pass the listener.

@JonasWanke
Copy link
Owner

An easy way to pass the listener to the widgets that need it (DateEvents and AllDayEvents) is to add additional parameters to the intermediate widgets that are being built, similar to how eventBuilder is passed down. Applying that idea to your example results in:

        Expanded(
          child: TimetableContent<E>(
            controller: controller,
            eventBuilder: eventBuilder,
            onCreateEvent: onCreateEvent,
          ),
        ),

That results in an additional parameter in a few intermediate widgets, but due to the large refactoring coming soon, we don't really need a more elegant way (like an InheritedWidget) for now.

DateEvents is the widget responsible for positioning the widgets of all part-day Events of a single day. Now that I think of it, we don't really need to go that far down. It's probably enough to pass the listener down through TimetableContent to MultiDateContent and add a GestureDetector around the StreamedDateEvents widget.

For the header and all-day events, the listener would get passed down through TimetableHeader to AllDayEvents (which lays out the widgets of all all-day Events) and a GestureDetector could be placed around _buildEventLayout(…).

Note: The term page used in a few places is the amount of days from epoch (January 1, 1970) to the currently focused date. It usually is a whole number, but has decimal digits while scrolling through dates.

@raLaaaa
Copy link
Contributor Author

raLaaaa commented Jun 19, 2020

DateEvents is the widget responsible for positioning the widgets of all part-day Events of a single day. Now that I think of it, we don't really need to go that far down. It's probably enough to pass the listener down through TimetableContent to MultiDateContent and add a GestureDetector around the StreamedDateEvents widget.

Currently making progress. Taps are received in DateEvents and give back the actual date. How ever I'm currently only getting back the current date (no time). If I understand your comment correctly we need to pass down that listener even more towards the events to get the actual selected timeframe? I think that would be pretty helpful.

Also I'm a bit confused since we defined OnCreateEventCallback = void Function(LocalDateTime start, bool isAllDay); with LocalDateTime start as parameter. Doesn't this force us to pass it down ahead of DateEvents?

@JonasWanke
Copy link
Owner

Looks good so far!

To get the time, we probably need to read the y-offset of the tap and calculate it manually. To get the offset, register onTapUp in the GestureDetector (which contains a TapUpDetails argument with a localPosition) instead of onTap, which doesn't have any information about where the tap occurred. (You can get the full height of the widget by wrapping the GestureDetector in a LayoutBuilder.)

The GestureDetector looks fine in its current position and doesn't need to be passed down any further. Sorry if my last comment was unclear :/

I'm not quite sure what you mean by your last question — we already have access to the respective date which just needs to be combined with the calculated time.

@raLaaaa
Copy link
Contributor Author

raLaaaa commented Jun 19, 2020

Ah! No worries. Thanks for your explanation. What caused confusion for me was the part with TapUpDetails. I did not know that I could get the localPosition of that. That's why I assumed we had to pass the listener down furthermore.

I think we almost made it for the timetable_content part.
I'd be interested in your feedback regarding my implementation:

 child: DatePageView(
        controller: widget.controller,
        builder: (_, date) {
          return LayoutBuilder(
          builder: (context, constraints) {
             return GestureDetector(
              behavior: HitTestBehavior.translucent,
              onTapUp: (details) {
                final cell = details.localPosition.dy / (((constraints.heightConstraints() / 24).maxHeight).floor());
                final time = DateTime(date.year, date.monthOfYear, date.dayOfYear, cell.toInt());
                final startTime = LocalDateTime.dateTime(time);
                print(startTime);
            //    widget.onCreateEvent(startTime, false);
              },
              child: StreamedDateEvents<E>(
                date: date,
                controller: widget.controller,
                eventBuilder: widget.eventBuilder,
              ));});
        },
      ),

This is the way I'm currently creating the start time of the tapped event. There is still a flaw because it is currently 1 hour off. Gonna checkout tomorrow why that is. Do you see any flaws or improvements we could do to calculate the tapped starting time (variable naming aside for now)?

Thanks for your awesome help so far.


My attempt was too calculate the height dimensions of each cell from 00:00 - 23:00 and then determine based on the tap position which cell was selected. But currently it results in inconsistencies so that i have two different timeframes in one cell.

The offset of 1 hour apparently origins by a flawed calculation of the tapped cell position. I also tried to floor() the calculation but that doesn't help much. I believe this might be related to ((constraints.heightConstraints() / 24).maxHeight).

EDIT:

Sorry for the spam I think I figured it out and find the problem.

TLDR: I think it's working now using. How ever the +1 offset still bothers me somehow:

final cell = details.localPosition.dy / ((constraints.maxHeight / 24).round());
final time = DateTime(date.year, date.monthOfYear, date.dayOfYear, cell.toInt() + 1);
final startTime = LocalDateTime.dateTime(time);
print(startTime);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Feature Type: :tada: New Features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants