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: ics as file #174

Closed
wants to merge 2 commits into from
Closed

feat: ics as file #174

wants to merge 2 commits into from

Conversation

atymic
Copy link
Contributor

@atymic atymic commented May 25, 2023

Context and Purposes

Sometimes, we want to be able to download a file rather than embedding a base64 link.
I used the existing options but would be open to doing it a different way if prefered.

@MehGokalp
Copy link

I was looking for this solution. Thanks

@UksusoFF
Copy link

UksusoFF commented Dec 8, 2023

Similar PR was closed with no activity: #139

So let's make some noise :)

Can you merge it?

@alies-dev @freekmurze

@atymic
Copy link
Contributor Author

atymic commented Jan 25, 2024

@freekmurze
@alies-dev
Any updates on this one?

@alies-dev
Copy link
Collaborator

alies-dev commented Jan 26, 2024

Hey
@atymic
@UksusoFF

Thank you a lot for your patience. I appreciate your help and passion to improve this package! 💪

A saw few PRs here, all of them tried to add more functionality to ICS generator by adding more options to the $options argument. There are a few types of problems:

  • violation of package principles (see the main readme)
  • trying to make ICS really big and complex. This package doesn't have a goal to be a comprehensive ICS generator, it's made to support basic ICS rules, also available in other generators. For advanced users, this package supports custom generators that you can use by calling with \Spatie\CalendarLinks\Link::formatWith()
  • mixing ICS components with presentation logic

This particular PR has the issue # 3. Today I found a time to solve it and created another PR #185, can you please review it guys and leave your feedback? Will it solve your goals?

@alies-dev alies-dev self-requested a review January 26, 2024 09:03
Copy link
Collaborator

@alies-dev alies-dev left a comment

Choose a reason for hiding this comment

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

mixing ICS components and presentation logic, see #174 (comment) for details.

@UksusoFF
Copy link

@alies-dev
Yep, #185 is great.

@atymic
Copy link
Contributor Author

atymic commented Jan 26, 2024

I will close this PR in favour of #185

@atymic atymic closed this Jan 26, 2024
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