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

Streamline LaTeX usage #8256

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

fneumann
Copy link
Contributor

@fneumann fneumann commented Oct 24, 2024

Feature Request:
https://docu.ilias.de/go/wiki/wpage_5614_1357

Description:
https://github.com/fneumann/ILIAS/blob/dev10-streamline-latex/components/ILIAS/UI/docs/mathjax.md

A review would be needed for components with the major changes:

  • alex40724 for COPage, Glossary, LearningModule, Repository, Survey, Wiki
  • klees or thibsy for UI
  • mjansenDatabay for Forum, Init, RTE
  • mbecker-databay, thojou or kergomard for T&A

Since changes in Accordion, Export, Utilities, Setup, SCORM2004 are trivial I think there is no need for a dedicated review there.

Some technical notes:

  • The introduction of the NPM dependency 'mathjax' was already approved in the JourFix with acceptance of the feature request.
  • MathJax is delivered as a distribution with already bundled components. The script components/ILIAS/UI/resources/js/MathJax/mathjax.js which is copied to the public folder sets the MathJax configuration in the browser and loads MathJax asynchronously.
  • The MathJax service is deleted. Most of the work is now done in the UI Framework. I followed the proposals made by thibsy and klees during preparation of the FR.
  • The Jour Fix decided that MathJax should be activated with a setting, like in former versions. This setting is read from the ILIAS config file by the setup of the UI framework and stored in the settings table. It is read in ilInitialisation::initUIFramework to create a data object implementing the interface \ILIAS\UI\Implementation\Render\MathJaxConfig which is provided for the UI framework.
  • The MathJaxConfig is also provided via DIC for the components to control the display of the MathJax hints, mostly below textarea form elements that support MathJax.

@fneumann fneumann added improvement dependencies Pull requests that update a dependency file php Pull requests that update Php code javascript Pull requests that update Javascript code labels Oct 24, 2024
@fneumann fneumann requested review from nhaagen and mjansenDatabay and removed request for nhaagen October 24, 2024 00:47
Copy link
Contributor

@mjansenDatabay mjansenDatabay left a comment

Choose a reason for hiding this comment

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

The changes in "my" components LGTM!

@Amstutz
Copy link
Contributor

Amstutz commented Oct 24, 2024

Hi @fneumann

Thx a lot for this big and important contribution. We looked over the UI related changes. We agree with the core concepts. However, there are public interface changes which need JF approval and several smaller things that need discussion and/or changes. This is more than can be achieved until tomorrow evening. Therefore, we are afraid, this will be too tight for the upcoming release. We would appreciate it if you contact us early, especially when we approach deadlines. We are willing to help push a large project over tight deadlines; however, we need to sync the workload if time-consuming tasks like this need fast processing.

Note, UI changes are release-independent. It could be possible to merge them, after the release date. However, this is not the case for the implementation of the underlying FR.

Thx a lot
@Amstutz, @klees, @thibsy

@mjansenDatabay
Copy link
Contributor

Thank you very much for your review at such short notice @Amstutz, @klees, @thibsy .

We will postpone the impl. to ILIAS 11.

Copy link
Contributor

@kergomard kergomard left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @fneumann

Even if this is not for ILIAS 10, I approve the changes for the Test, this way you do not need for us, once this is done.

Best,
@kergomard

@matthiaskunkel
Copy link
Member

I remove the 'dependency' tag for this PR as it is about code changes for 11 and not related to a dependency we have to discuss at the Dependency JF. For the MathJax dependency there should be a proper PF.

@matthiaskunkel matthiaskunkel removed the dependencies Pull requests that update a dependency file label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement javascript Pull requests that update Javascript code php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants