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

fix(core feature-detection): Fix loading of modernizr script. #1231

Merged
merged 3 commits into from
Jan 28, 2025

Conversation

thet
Copy link
Member

@thet thet commented Jan 28, 2025

  1. fix(core feature-detection): Fix loading of modernizr script.

In some situations loading of the modernizr.min.js script failed because the base url could be incorrect. That is fixed with a non-IE compatible method of getting the current's script URL, which is safe to use. The problem surfaced in Plone while loading the Patternslib library and in between the protect.js script was loaded. The base url was calculated for the protect.js script and the modernizr script could not be loaded.

2)‌ maint(core log): Console.debug is not deprecated. Just use it.

  1. fix(core registry): Do nothing with Patterns without a trigger.
    Patterns without a trigger broke the registry scan method. Now they don't.

In some situations loading of the modernizr.min.js script failed because
the base url could be incorrect. That is fixed with a non-IE compatible
method of getting the current's script URL, which is safe to use.
The problem surfaced in Plone while loading the Patternslib library and
in between the protect.js script was loaded. The base url was calculated
for the protect.js script and the modernizr script could not be loaded.
@thet thet force-pushed the fix-loading-modernizr branch from 89141ae to c110df8 Compare January 28, 2025 11:23
@cornae
Copy link
Member

cornae commented Jan 28, 2025

Alternately, we could we could start thinking of phasing out Modernizr. I've removed most if not all dependencies over the past years.

@thet
Copy link
Member Author

thet commented Jan 28, 2025

@cornae sounds good, I'm 👍 for phasing that out.
Maybe the 9.10 series would be a good fit.
If we agree on that I could make another alpha after that one and remove Modernizr.

I will still proceed an make an alpha version with this fixes.

@thet
Copy link
Member Author

thet commented Jan 28, 2025

@cornae there is also this fix:

"fix(core registry): Do nothing with Patterns without a trigger."

I found it while integrating the whole Patternslib bundle into Plone.
The source of the error was the focus pattern. This one does not have any trigger set, but initializes by it's own. So I think that the focus patterns still works. Please test it, if you have a usecase.

However, I think we can easily deprecate / remove the focus pattern.
If it's purpose is to set a class if one of the child elements has a focused state, we can nowadays do it like so:

:has(:focus) {}

https://developer.mozilla.org/en-US/docs/Web/CSS/:has

or even like so (as I just found out):

:focus-within {}

https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-within

@thet thet merged commit 0b60791 into master Jan 28, 2025
1 check passed
@thet thet deleted the fix-loading-modernizr branch January 28, 2025 19:32
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