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

IGAPP-1152: Export events api client #831

Merged
merged 22 commits into from
Feb 13, 2023

Conversation

Cmd190
Copy link
Contributor

@Cmd190 Cmd190 commented Dec 14, 2022

This pull request also contains IGAPP-1129.
How to test it: Go to Events => Click on Item => click on export button

Import Google Kalendar: Settings -> Import & Export
For other applications: Just open ics file

@steffenkleinle steffenkleinle changed the title Igapp 1152 export events api client IGAPP-1152: Export events api client Dec 15, 2022
Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

Really nice work. Tested on apple calendar and google calendar.
I have commented some minor adjustments

Copy link
Member

@steffenkleinle steffenkleinle left a comment

Choose a reason for hiding this comment

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

Tested on web, works fine. In the designs the background of the button is grey instead of yellow, why did you choose yellow?

@f1sh1918
Copy link
Contributor

f1sh1918 commented Jan 31, 2023

Tested on web, works fine. In the designs the background of the button is grey instead of yellow, why did you choose yellow?

Just added comment to design, since we should stick to our standard <TextButton> if possible and they are yellow. I would at least suggest to use same style everywhere

Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Looks good, tested in Firefox. Have to concur with Steffen though, in the design the button is grey and wider. Nvm, just saw your previous comment about it.

Copy link
Member

@ztefanie ztefanie left a comment

Choose a reason for hiding this comment

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

Looks good, testes on Chrome

only one small thing:
Line length is too long for ical format (but it is only warning not error level)
https://icalendar.org/iCalendar-RFC-5545/3-1-content-lines.html

@f1sh1918 f1sh1918 requested a review from ztefanie February 7, 2023 12:35
@f1sh1918
Copy link
Contributor

hm do you think we should force adding new lines? I imported in several calendars and couldn't find any issue. It will look very odd to randomly add news lines every ~20 chars @SteffiStuffel

@f1sh1918 f1sh1918 force-pushed the IGAPP-1152-export-events-api-client branch from 8c1e05e to 3c2d232 Compare February 10, 2023 17:51
@f1sh1918 f1sh1918 enabled auto-merge February 13, 2023 18:10
@f1sh1918 f1sh1918 merged commit 4105597 into main Feb 13, 2023
@f1sh1918 f1sh1918 deleted the IGAPP-1152-export-events-api-client branch February 13, 2023 18:46
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.

5 participants