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

Add dependency on fullcalendar_library #25

Open
wants to merge 1 commit into
base: gh-8.x-1.x
Choose a base branch
from

Conversation

acrollet
Copy link
Contributor

Fixes #7. (And possibly #16)

@acrollet
Copy link
Contributor Author

Hmmm, don't review just yet - not working as well as I thought ;p

@acrollet
Copy link
Contributor Author

Ok, this is ready for a look - be sure you're using 8.x-1.0-beta3 of fullcalendar_library

@acrollet
Copy link
Contributor Author

Hi @dakala, any update on this? :)

@dakala
Copy link
Owner

dakala commented Feb 16, 2018

@acrollet Apologies for the silence. Loading external libraries via CDN is definitely a good feature especially in production. However, there are instances when a local copy of a library is indispensable.

That's why I prefer an approach similar to Bootstrap - supporting both with an option to toggle settings between them. See the screenshots in case you don't use Bootstrap.

What do you think?

bootstrap-settings

bootstrap-settings2

@acrollet
Copy link
Contributor Author

Sorry if it's not clear, but a local copy of the library will always be used if present - the module only pulls from a CDN if it is not found.

@acrollet
Copy link
Contributor Author

note: the site status report notes when a CDN is used for the library and provides instructions of where to store the local copy...

@dakala
Copy link
Owner

dakala commented Feb 16, 2018

Great. But can't we download the library too? Core does something similar withcore.libraries.yml. And a setting where you opt to use CDN or no CDN.

@acrollet
Copy link
Contributor Author

@dakala can you expand on what you mean by downloading the library?

@dakala
Copy link
Owner

dakala commented Feb 16, 2018

In core.libraries.yml you'll find third-party libraries e.g. backbone, domready etc. This is what the ckeditor looks like:

ckeditor:
  remote: https://github.com/ckeditor/ckeditor-dev
  version: "4.7.2"
  license:
    name: GNU-GPL-2.0-or-later
    url: https://github.com/ckeditor/ckeditor-dev/blob/4.7.2/LICENSE.md
    gpl-compatible: true
  js:
    assets/vendor/ckeditor/ckeditor.js: { preprocess: false, minified: true }

We should be able to do something like this for fullcalendar. I'll try it tomorrow as I'm not 100% sure if this works like make files in D7

@dakala
Copy link
Owner

dakala commented Feb 17, 2018

@acrollet Looks like handling of 3rd-party libraries isn't as straightforward as I tried to suggest yesterday.

https://www.drupal.org/node/2605130
https://www.drupal.org/project/drupal/issues/2873160

Other projects are trying different solutions e.g. Chosen (http://cgit.drupalcode.org/chosen/tree/README.txt?h=8.x-2.x)

However, I'm still in favour of being able to switch between local and CDN copy of the library like Bootstrap.

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.

2 participants