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

SemanticResultFormats (KM-A) #727

Merged
merged 4 commits into from Sep 21, 2022

Conversation

thomas-topway-it
Copy link
Contributor

fix issue #724 (carousel not working)

krabina and others added 4 commits September 20, 2022 11:46
prepare for better accessibility by adding role and tabindex to div for views selector
@codecov-commenter
Copy link

Codecov Report

Merging #727 (af32d04) into master (ad5ea25) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #727   +/-   ##
=========================================
  Coverage     39.31%   39.31%           
  Complexity     2046     2046           
=========================================
  Files            75       75           
  Lines          7233     7233           
=========================================
  Hits           2844     2844           
  Misses         4389     4389           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@gesinn-it-gea
Copy link
Member

To me this looks like the embedded jquery has been patched. This would break again on jquery update.

@krabina
Copy link
Contributor

krabina commented Sep 21, 2022

What is the alternative?

@gesinn-it-gea
Copy link
Member

... well, the original source https://github.com/Wilto/Dynamic-Carousel isn't maintained anyway. So risk of breaking again is almost 0. An alternative would be to use an alternate plugin that is maintained while ideally keeping the SMW interface unchanged :-)

@krabina
Copy link
Contributor

krabina commented Sep 21, 2022

Yes, I see. Well then the scope of this PR is clear: to fix the bug when bootstrap is present. Maybe we should open another issue that SRF is using an outdated plugin.

@malberts
Copy link
Contributor

malberts commented Sep 21, 2022

I think this is fine (and roughly what I had in mind in ProfessionalWiki/chameleon#346 (comment)).

The upstream plugin is dead and if there happens to be an update there then the diff will at least show the name changed again and git blame should trace it back to this issue.

@gesinn-it-gea
Copy link
Member

... if you look at the timestamps, most of the js has not been touched the last years. As long as they work, it's fine. But fixing security issues or keeping things running with more recent versions of other components is another story. Having an additional issue to follow-up on this one is definitely the right direction.

To my opinion, this PR can be merged to get your original problem solved.

@krabina
Copy link
Contributor

krabina commented Sep 21, 2022

#728

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

5 participants