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

Compare date poll with user's calendar #747

Closed
wants to merge 23 commits into from
Closed

Conversation

dartcafe
Copy link
Collaborator

First attempt
grafik

grafik

@dartcafe
Copy link
Collaborator Author

Currently blocked by nextcloud/server#18679

@dartcafe dartcafe added this to the next milestone Jan 15, 2020
@dartcafe dartcafe self-assigned this Jan 15, 2020
@jancborchardt
Copy link
Member

jancborchardt commented Jan 15, 2020

Very nice! :)

  • Let’s use the calendar icon instead of an error icon
  • Icon can be white instead of black on the orange color
  • If we know the time of the poll option, we should list the conflicting event directly, and only the relevant one. So right of the icon:

10:00 – 11:00
Design team meeting

@dartcafe
Copy link
Collaborator Author

dartcafe commented Jan 18, 2020

If we know the time of the poll option, we should list the conflicting event directly, and only the relevant one. So right of the icon:

10:00 – 11:00
Design team meeting

I decided to put it into the tooltip, because we could have a situation like this:
grafik

There will be no space to add this additional information, IMHO.

@jancborchardt
Copy link
Member

If we show it in the row where you can reply, it might make a bit more sense than in the top bar?
Then instead of the icon, we can say "Calendar conflict" or something short like that which is more understandable.

@dartcafe
Copy link
Collaborator Author

we can say "Calendar conflict" or something short like that which is more understandable.

Hmm. I would prefer the andon way. 😄

@v1r0x
Copy link
Collaborator

v1r0x commented Feb 6, 2020

Great feature! Looks already very nice!

I'd also move it to the users row, so it's better visible while going through the options. I have no real opinion whether using tooltip or not, but I'd suggest focusing on the important stuff and only show the name of the event (maybe from which calendar?) and a "... and 3 more events", if there are several conflicting events. Without a tooltip we could add more info to all of these events to a tooltip.

Here's a screenshot from a version without tooltip and icon moved down to the users row

polls-calendar

@dartcafe
Copy link
Collaborator Author

dartcafe commented Feb 7, 2020

In my opinion the text will break the design, if there are a lot of options, so I would keep the tooltip. Regarding moving the icon, I will try some variants in a few weeks. Nevertheless, we are dependent on the release of the PR in the server repo.

@dartcafe
Copy link
Collaborator Author

dartcafe commented Feb 7, 2020

And BTW: Any idea, how to create a calendar event, without building a dav client in polls? I looked at the implementation in the mail app. Not what I expected.

@dartcafe
Copy link
Collaborator Author

@georgehrke @tcitworld Is there a simple way to add an event to the calendar, without building a DAV client, like it seems that is done in the mail app?

@tcitworld
Copy link
Member

Managing calendars can be done with OCP\Calendar but for managing events I'm afraid you currently need to hook in directly into OCA\DAV\CalDAV\CalDavBackend.

@dartcafe
Copy link
Collaborator Author

@tcitworld any walkthru or pattern available?

but for managing events I'm afraid you currently need to hook in directly into OCA\DAV\CalDAV\CalDavBackend.

@tcitworld
Copy link
Member

tcitworld commented Feb 19, 2020

What you want is just calling CalDavBackend->createCalendarObject($calendarId, $objectUri, $calendarData) with iCalendar data for $calendarData. See the tests for instance: https://github.com/nextcloud/server/blob/master/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php#L206-L232

@dartcafe
Copy link
Collaborator Author

@tcitworld Thanks. I think that helps me getting into it.

@jancborchardt
Copy link
Member

In my opinion the text will break the design, if there are a lot of options, so I would keep the tooltip. Regarding moving the icon, I will try some variants in a few weeks. Nevertheless, we are dependent on the release of the PR in the server repo.

Since the info about the conflict is the most important thing here, and tooltips are not very discoverable, it should be directly shown. Even just "Conflicts with Event name" would be great, the calendar icon on orange is not really needed cause the text already says it.

@dartcafe
Copy link
Collaborator Author

Since the info about the conflict is the most important thing here, and tooltips are not very discoverable, it should be directly shown. Even just "Conflicts with Event name" would be great, the calendar icon on orange is not really needed cause the text already says it.

I would say, at first sight it is not important to know which calendar entry causes the conflict. In fact the time before and after the poll date is interesting as well, because, you have to think about travel times or similar.

@dartcafe dartcafe closed this Mar 31, 2020
@dartcafe
Copy link
Collaborator Author

I will try some mockups to illustrate the problem I see.

@dartcafe dartcafe reopened this Mar 31, 2020
@dartcafe dartcafe changed the base branch from master to dev-1.5 August 15, 2020 18:22
@dartcafe
Copy link
Collaborator Author

Closing this, as the PR is rather old. I ported the attempt to #1056

@dartcafe dartcafe closed this Aug 17, 2020
@dartcafe dartcafe deleted the accessCalendar branch August 17, 2020 09:08
@dartcafe dartcafe modified the milestones: next, 1.5 Sep 21, 2020
@dartcafe dartcafe mentioned this pull request Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants