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

A few enhancements #36

Merged
merged 7 commits into from
Nov 1, 2021
Merged

Conversation

aloxe
Copy link
Contributor

@aloxe aloxe commented Oct 31, 2021

I faced a few problems while installing and configuring the plugin on grav. I think the following changes will fix them and make the plugin more user friendly.

  • request for jQuery
  • space left around calendar ics urls
  • find correct URL for proxy.php even when grav isn't installed at the root
  • error when language not set
  • legend always displayed
    Each change is on a separate commit.

@wernerjoss wernerjoss merged commit a13f467 into wernerjoss:master Nov 1, 2021
@wernerjoss
Copy link
Owner

@aloxe :
thanks for taking the time to improve this plugin 👍
I just created a new release (0.2.11) BEFORE I saw your pull request, so that is not already in 0.2.11.
from what I saw so far, your changes look fine.
I will now try the result and create another new release when everyting is ok, but let you know before.

@wernerjoss
Copy link
Owner

... as commented, I'm back again:
the automatic CORS Url detection from your patch did not work for me, see attached screenshot.
so I decided to do this using getAbsolutePath() - then it is ok.
please check/comment.
grafik

@aloxe
Copy link
Contributor Author

aloxe commented Nov 3, 2021

The problem with getAbsolutePath() is it is using the url of the current page to find the plugin path and this only works when the calendar page is not nested. If you move your test calendar in a folder, it will not find the right proxy url.

In your screenshot I see that in your test linktags doesn't grab the <link rel="stylesheet"> from the plugin which doesn't look normal. Is it in the page code header?

@wernerjoss
Copy link
Owner

wernerjoss commented Nov 5, 2021 via email

@wernerjoss
Copy link
Owner

wernerjoss commented Nov 5, 2021 via email

@aloxe
Copy link
Contributor Author

aloxe commented Nov 5, 2021

So @wernerjoss that works for you too right?

@wernerjoss
Copy link
Owner

wernerjoss commented Nov 5, 2021 via email

@wernerjoss
Copy link
Owner

wernerjoss commented Nov 6, 2021

FYI, I have now changed the code on my live site,
so you can see in console see why linktagurl is not working, but the page does not crash

@aloxe
Copy link
Contributor Author

aloxe commented Nov 6, 2021

no, it is just as I show in my screenshot before. ATM it can be seen on my live site at https://hoernerfranzracing.de/werner/kalender including some console.log() statements. I'll let it in this state until tomorrow morning for reference, then I will restore again my version with getAbsolutePath() which works for me :-)

What I can see is that your calendar is not loading the css stylesheet that comes with the plugin. I guess this is a use case we should take into account. I'll work on something that passes {{ base_url_absolute }} to the DOM, I thing we can't avoid it.

@wernerjoss
Copy link
Owner

wernerjoss commented Nov 6, 2021 via email

@aloxe
Copy link
Contributor Author

aloxe commented Nov 6, 2021

Am Samstag, 6. November 2021, schrieb wernerjoss:

I have done that in git master, works for me here. (code still needs formatting, though) please check / compare with what you intend to do.

This is exactly what I intended to do (except using the twig variable). Since your example was still having CORS API URL in 404. I submitted PR #37 to fix it.

@aloxe aloxe deleted the a-few-enhancements branch November 6, 2021 23:51
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

2 participants