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

feat: Added options for Outlook, Yahoo to add to calendar #5541

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

codedsun
Copy link
Contributor

@codedsun codedsun commented Nov 9, 2020

Fixes #5072

Short description of what this resolves:

Add options for add to calendar

Changes proposed in this pull request:

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@auto-label auto-label bot added the feature label Nov 9, 2020
@vercel
Copy link

vercel bot commented Nov 9, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/3e1vax1q4
✅ Preview: https://open-event-frontend-git-calendar.eventyay.vercel.app

@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #5541 (727bb65) into development (007805a) will increase coverage by 0.44%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5541      +/-   ##
===============================================
+ Coverage        23.26%   23.71%   +0.44%     
===============================================
  Files              494      498       +4     
  Lines             5226     5255      +29     
  Branches            39       44       +5     
===============================================
+ Hits              1216     1246      +30     
+ Misses            4004     4003       -1     
  Partials             6        6              
Impacted Files Coverage Δ
app/components/public/add-to-calender.ts 4.54% <0.00%> (ø)
app/utils/computed-helpers.js 19.56% <0.00%> (-1.72%) ⬇️
app/models/social-link.js 0.00% <0.00%> (ø)
app/routes/events/view/tickets/attendees/list.js 0.00% <0.00%> (ø)
app/components/forms/wizard/other-details-step.js 0.00% <0.00%> (ø)
app/utils/dictionary/social-media.ts 100.00% <0.00%> (ø)
app/components/widgets/forms/link-field.ts 75.00% <0.00%> (ø)
app/components/widgets/forms/social-link-field.ts 75.00% <0.00%> (ø)
app/components/events/view/overview/event-apps.js 100.00% <0.00%> (ø)
app/components/smart-overflow.js 100.00% <0.00%> (+38.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 007805a...4fcf801. Read the comment docs.

@iamareebjamal
Copy link
Member

Think about this for a moment. If it was working already, why would I have commented it out?

image

If it was just that, uncommenting some lines, I would have done it in the PR myself and not reopened the issue

@iamareebjamal
Copy link
Member

@codedsun
Copy link
Contributor Author

@iamareebjamal For local it was working fine, I will surely look into this

@codedsun
Copy link
Contributor Author

codedsun commented Nov 10, 2020

@iamareebjamal - as the description of the event goes too long, the add to calendar on outlook crashes. We need to have some limit of the description text when adding the event to outlook.

Check this https://open-event-frontend-kpkdr6kyl.vercel.app/e/ae73f280

@iamareebjamal
Copy link
Member

Then please add the limit

@@ -49,7 +49,7 @@ export default class AddToCalender extends Component<Args> {
const { event } = this.args;
const startTime = this.startsAt.utc().format('YYYY[-]MM[-]DDTHH[:]mm[:]SS[Z]');
const endTime = this.endsAt.utc().format('YYYY[-]MM[-]DDTHH[:]mm[:]SS[Z]');
return `https://outlook.live.com/calendar/0/deeplink/compose?path=/calendar/action/compose&rru=addevent&subject=${event.name}&startdt=${startTime}&enddt=${endTime}&body=${event.description}&location=${this.args.location}`;
return `https://outlook.live.com/calendar/0/deeplink/compose?subject=${event.name}&startdt=${startTime}&enddt=${endTime}&body=${(event.description).substring(0,80)}&location=${this.args.location}`;
Copy link
Member

Choose a reason for hiding this comment

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

80 is quite small, is it max?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's no where documented, I will have to try and test what is the limit.

Copy link
Member

Choose a reason for hiding this comment

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

Also event name and description show + instead of space. Check if there is an easy fix, otherwise it is fine

@codedsun
Copy link
Contributor Author

codedsun commented Nov 11, 2020

@iamareebjamal The outlook is crashing for 1500+ char limits. The url should max contain 2048 characters (GET) request. IMO it was crashing because of this. So therefore I have truncated the string to 1000characters.

and the + in title is resolved

@iamareebjamal iamareebjamal merged commit 7e7b0f7 into fossasia:development Nov 11, 2020
@iamareebjamal
Copy link
Member

How was + issue resolved

@codedsun
Copy link
Contributor Author

codedsun commented Nov 11, 2020

How was + issue resolved

I was creating the event on the browser and check how the URl Address changed with enteries.
I removed path=/calendar/action/compose&rru=addevent as it was found no where in the URL address in the process of creating the event in my calendar and it helped me resolved the issue.

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

Successfully merging this pull request may close these issues.

Feature: Add to Calendar
2 participants