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

UI Components, New Glyphs for il-icons fonts see #41767 #7925

Closed
wants to merge 640 commits into from

Conversation

Amstutz
Copy link
Contributor

@Amstutz Amstutz commented Aug 9, 2024

Addition of 6 new Glyphs with description and font changes, see https://mantis.ilias.de/view.php?id=41767 for details.

@yvseiler
Copy link
Collaborator

yvseiler commented Aug 20, 2024

Hi @Amstutz

Thank you a lot for contributing to ILIAS. This significantly improves the glyph database.

I just have one question according to the Files changed:

Please answer the following questions:

  • Why is there no entry in the language files de and eng for the preview glyph? Does it already exist?

kindly,
@yvseiler

@Amstutz
Copy link
Contributor Author

Amstutz commented Aug 27, 2024

Why is there no entry in the language files de and eng for the preview glyph? Does it already exist?

Because it already exists.

@klees and @thibsy , this is ready for review. If good, it would need to go to the JF. See mantis isssue above for details. Trunk only.

@Amstutz Amstutz assigned klees and thibsy and unassigned yvseiler Aug 27, 2024
Copy link
Contributor

@thibsy thibsy left a comment

Choose a reason for hiding this comment

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

Hi @Amstutz and @yvseiler,

Thanks a lot for your contribution to the UI components!

The factory descriptions are plausible and can be presented at the next JF. There are some minor changes before we can merge this afterwards, but they are not important for JF.

Please implement the following changes:

  • Unit tests: the unit tests are currently failing. Please look into this.
  • ilias_de.lang: the german translation of common#:#tile_view is in english. Please translate this variable.
  • @inheritDoc: this PHPDoc annotation is obsolete and can be removed from the new methods in the Glyph\Factory implementation.

Kind regards,
@klees and @thibsy

@matthiaskunkel
Copy link
Member

Jour Fixe, 30 SEP 2024: We highly appreciate this suggestion and accept the PR for ILIAS 10/trunk.

dsstrassner and others added 17 commits September 30, 2024 15:21
…ering and range. (ILIAS-eLearning#8106)

* Fix https://mantis.ilias.de/view.php?id=41328
* Fix https://mantis.ilias.de/view.php?id=42095
* Remove Table\Data::withNumberOfRows()
* Replace IRSS usage of removed method
(cherry picked from commit c396f3e)
(cherry picked from commit 99ea744)
fhelfer and others added 26 commits October 23, 2024 10:35
UnitTests have been modified in LTIProvider to disable them until the new required dependencies are fully integrated into the LTI components.
This commit extracts the code to conditionally prepend a
reply-prefix to the subject of the related parent posting
from class `ilObjForumGUI` to a separate class.
Furthermore, a unit test is added which tests different
variants of language variables defined as "reply prefix"
(e.g. "Re: "), "repeated reply prefix" and
"optimized repeated reply prefix" (e.g. "Re (4)").

The main challenge is that the prefixes are defined
in language files and can be changed in the advanced
language administration. The code has to be robust
against these changes. However, the prefix-builder
does not need to support any arbitrary translations
(shit in, shit out).

Finally: The prefix-builder should be "multibyte safe",
so it uses the `mb_*` PHP functions if available.

See: https://mantis.ilias.de/view.php?id=42322

(cherry picked from commit 2316d1d)

(cherry picked from commit 575e2d0)
Skipped LTIProvider component UnitTests temporarily
This commit extracts the code to conditionally prepend a
reply-prefix to the subject of the related parent posting
from class `ilObjForumGUI` to a separate class.
Furthermore, a unit test is added which tests different
variants of language variables defined as "reply prefix"
(e.g. "Re: "), "repeated reply prefix" and
"optimized repeated reply prefix" (e.g. "Re (4)").

The main challenge is that the prefixes are defined
in language files and can be changed in the advanced
language administration. The code has to be robust
against these changes. However, the prefix-builder
does not need to support any arbitrary translations
(shit in, shit out).

Finally: The prefix-builder should be "multibyte safe",
so it uses the `mb_*` PHP functions if available.

See: https://mantis.ilias.de/view.php?id=42322

(cherry picked from commit 2316d1d)

(cherry picked from commit 575e2d0)
This commit extracts the code to conditionally prepend a
reply-prefix to the subject of the related parent posting
from class `ilObjForumGUI` to a separate class.
Furthermore, a unit test is added which tests different
variants of language variables defined as "reply prefix"
(e.g. "Re: "), "repeated reply prefix" and
"optimized repeated reply prefix" (e.g. "Re (4)").

The main challenge is that the prefixes are defined
in language files and can be changed in the advanced
language administration. The code has to be robust
against these changes. However, the prefix-builder
does not need to support any arbitrary translations
(shit in, shit out).

Finally: The prefix-builder should be "multibyte safe",
so it uses the `mb_*` PHP functions if available.

See: https://mantis.ilias.de/view.php?id=42322

(cherry picked from commit 2316d1d)

(cherry picked from commit 575e2d0)
… new_glyphs_10

* 'new_glyphs_10' of https://github.com/Amstutz/ILIAS:
  UI Components, New Glyphs for il-icons fonts see #41767
@Amstutz Amstutz marked this pull request as draft October 23, 2024 12:52
@Amstutz Amstutz closed this Oct 23, 2024
@Amstutz Amstutz deleted the new_glyphs_10 branch October 23, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.