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

AddEvent styling, behaviour and some more #33

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

Conversation

commonpike
Copy link

@commonpike commonpike commented Dec 6, 2018

As http://addtocalendar.com recently became commercial, and very expensive too, #justSayin, I wanted another widget for that functionality. ouical.js came the closest, and I decided to give it an overhaul.

Did this

  • added AddToCalendar styling, with a popup
  • added AddToCalendar html instantiation - no js required
  • added IANA timezones support
  • added 'all day' support (Add Day Events #8,Fix yahoo allday #29)

then went through a few of the pending PRs and issues here and fixed some

But along the way I also shuffled the js and css files, renamed the whole project to "OUI2 Add To Calendar". So I am not sure how mergeable this is at this point.

Let me know if you are interested!

@carlsednaoui
Copy link
Owner

Hi @commonpike thanks a lot for the PR — I've been doing a pretty bad job at keeping up with this (and a few other) libraries as of late. Let me review the PR in the coming weeks and see if we can get this merged!

@commonpike
Copy link
Author

Great, and I sympathize ... you built a great thing and then it haunts you for maintenance :-)

You can quickly see what it looks like now at
https://commonpike.github.io/add-to-calendar-buttons/example.html
https://commonpike.github.io/add-to-calendar-buttons/tester.html
https://commonpike.github.io/add-to-calendar-buttons/generator/generator.html

@commonpike
Copy link
Author

commonpike commented Dec 21, 2018

But, really a lot has changed. The 'api' should be bwc, but people who have this in a bower or composer setup might be shocked to get a very different design suddenly. It may break things on their end.

If you like it, you could also consider setting up a new repo (like add-to-calendar-buttons-2 or just add-to-calendar) and moving forward from there.

@carlsednaoui
Copy link
Owner

@commonpike, would you be open to me adding you as a maintainer to this project and you can merge this in?

I'd feel comfortable releasing a v2 that comes with breaking changes. Thoughts?

@LittleDogFido
Copy link

I've been trying to get this project working for my needs and was glad to see someone had decided to give it an overhaul. The primary issue I have, the widget not working in IE or Edge when it comes to downloading the .ics, still persists. My own skills are relatively limited at the moment so I'm struggling to find a workaround my own. I was wondering if this project was something you planned on continuing work on, and if IE/Edge compatibility would be a part of that. I don't see any reason it SHOULDN'T work with either Microsoft browser as as far as I know, the "download" attribute is at least supported in Edge. In my research of similar issues I found Blob used a lot with client-side generated files but wasn't sure if it would applicable in this case.

@commonpike
Copy link
Author

@commonpike, would you be open to me adding you as a maintainer to this project and you can merge this in?

@carlsednaoui - (sorry missed this notification) - yes go ahead. I'm not sure if I can spend more time on it than you can, though.

I'd feel comfortable releasing a v2 that comes with breaking changes. Thoughts?

My thoughts would be, if someone has this included through composer or bower, and they'd update, things could break :-) So if you would, at least keep an old version (tag or branch) alive they can cling their installation on to. I would do a 'version1' branch I think.

@commonpike
Copy link
Author

@LittleDogFido - re "the widget not working in IE or Edge when it comes to downloading the .ics" - is that an issue in the issue tracker ? Do you still have that issue here - https://commonpike.github.io/add-to-calendar-buttons/tester.html ?

@LittleDogFido
Copy link

LittleDogFido commented May 21, 2019

It is in the issue tracker, yes. However, the link provided to the suggested fix there does not do anything to solve the issue for IE and Edge. I unfortunately still have the issue with the tester link you provided, trying it a couple different machines to make sure it wasn't something wrong with my browser settings.

On a side note, I noticed that you replied to carlsednaoui as well. Unfortunately, in my researching of this project I discovered that he tragically passed away a few months ago.

@commonpike
Copy link
Author

It is in the issue tracker, yes.

can you point me to that ?

On a side note, I noticed that you replied to carlsednaoui as well. Unfortunately, in my researching of this project I discovered that he tragically passed away a few months ago.

what ??
https://www.mailcharts.com/in-memoriam-carl-sednaoui

plane crash.

boy, that is sad. ... I dug around in his code so deep, I have the feeling I knew him. Good code. Taught me something.

well, this pull request is going nowhere, then. hm..

@LittleDogFido
Copy link

#15

If I'm understanding things correctly, JulienV seems to refer people to his Chrome fix for the issue but it doesn't have any effect on Internet Explorer or Edge. It SHOULD do the trick for Edge as far as I can tell, since the download attribute is supported, but nothing happens on click in my testing. It's a fantastic project but for my application, most users will be using IE or Edge so until I can find a work-around it won't be a ton of use to me personally.

I'm pretty early in the my development of my own skill set in this area, so immersing myself in his work has been a fantastic learning experience for me. I'm very grateful for what he has contributed.

@commonpike
Copy link
Author

@LittleDogFido I saw that but couldn't read what people were trying to say. I have this in production and tested and working for IE and Edge (as far as I know). Can you explain in more detail what the problem is, here : commonpike#1 ?

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.

None yet

4 participants