[Fix] 0045422: Use ButtonContextRenderer for Glyph in Filter and Mode…

[fhelfer]

Error: The element “a” must not appear as a descendant of the “button” element.
https://mantis.ilias.de/view.php?id=45422

Glyphs with action were rendered as and ended up inside ,
which is invalid. Extend USE_BUTTON_CONTEXT_RENDERER_FOR with
StandardFilterContainerInput and ModeViewControl so glyphs render
as in these contexts.

Problem

• Validator: <a> als Nachfahre von <button> ungültig (z. B. Filter, Dashboard ViewControlMode).

Lösung

• GlyphRendererFactory: StandardFilterContainerInput und ModeViewControl zu USE_BUTTON_CONTEXT_RENDERER_FOR hinzufügen → Glyphe als span in diesen Kontexten.

[Annett7811]

Hello!

From an accessibility perspective, this looks good. Thank you very much for your work.

Best regards,

Annett

[oliversamoila]

Hello @fhelfer
When I go to Mail » Inbox as described in the referenced Mantis issue and use the HTML validator, I still get errors of the type The element a must not appear as a descendant of the button element..
However, I consider these to be completely misleading, as they refer to 65115835.” ><a class="glyph" aria-label=“Tile View”>↩<span.
But: There is no ViewControl with these glyphs on these screens, nor are there any other places.

Do you @Annett7811 have these messages?

I would actually like to hand this over to Thibeau, but I'm stuck on it.

Best regards,
Oliver

[Annett7811]

Hello @oliversamoila!

No, I don't get this error message in combination with a button element.

Best regards,
Annett

[oliversamoila]

Thanks @Annett7811
I'm forwarding this to @thibsy for code review. He may contact you with change requests or incorporate the changes directly (That includes cherry picking too.)

Kind regards,
@oliversamoila (as UI coordinator)

[dsstrassner]

Das liest sich wie die falsche Vorlage für @Annett7811 😉

[oliversamoila]

Hey @dsstrassner, das passt soweit ;)
Der Text 'contact you with change requests' richtete sich aber potenziell eher an Fabian.
Also ja, das geht sprachlich schöner, aber wird funktionieren … hoffe ich ^^

[dsstrassner]

Nicht das Annett Angst bekommt das sie selbst den PR anpassen muss. Harhar. 🙃

[thibsy]

Hi @fhelfer,

Thx a lot for your contribution to the UI framework!

Please help me answer the following questions about your changes:

• ModeViewControl: you say this component is also affected by the same issue. However, I cannot find a glyph usage in the ViewControl\Mode or the Input\ViewControl\Mode rendering. Are you sure this is relevant?

Please implement the following changes:

• False-positives: I think once the StandardFilterInput is added to the USE_BUTTON_CONTEXT_RENDERER_FOR constant, every input that is contained inside such a filter will be rendered using this context renderer. I believe this creates false-positives. Please treat this component differently, by checking explicitly if its the current component, not just contained in the rendering chain. Maybe we should introduce a separate constant with appropriate documentation for this =).

Kind regards,
@thibsy (as UI coordinator)

[fhelfer]

• ModeViewControl: you say this component is also affected by the same issue. However, I cannot find a glyph usage in the ViewControl\Mode or the Input\ViewControl\Mode rendering. Are you sure this is relevant?

Hey @thibsy,
this is used here

ILIAS/components/ILIAS/Dashboard/Block/classes/class.ilDashboardBlockGUI.php

Lines 394 to 396 in 93771e0

	$this->addPresentation(
	$this->ui->renderer()->render($this->ui->factory()->symbol()->glyph()->{$presentation . 'View'}()),
	$this->ctrl->getLinkTarget($this, 'changePDItemPresentation'),

which later gets rendered into a ViewControl Mode Switch here

ILIAS/components/ILIAS/Container/Block/class.ilBlockGUI.php

Lines 737 to 769 in bfc1335

	public function getViewControlsForPanel(): array
	{
	$viewControls = [];
	if (count($this->presentations) > 1) {
	$presentation = $this->factory->viewControl()->mode(
	$this->presentations,
	'label'
	)->withActive($this->activePresentation);
	$viewControls[] = $presentation;
	}
	if ($this->sort_options !== []) {
	if (! array_key_exists($this->activeSortOption, $this->sort_options)) {
	$this->activeSortOption = current(array_keys($this->sort_options));
	}
	$sortation = $this->factory->viewControl()->sortation(
	$this->sort_options,
	$this->activeSortOption
	)->withTargetURL(
	$this->sort_target,
	'sorting'
	);
	$viewControls[] = $sortation;
	}
	if ($this->getPresentation() === self::PRES_SEC_LIST) {
	$pg_view_control = $this->getPaginationViewControl();
	if ($pg_view_control !== null) {
	$viewControls[] = $pg_view_control;
	}
	}
	return $viewControls;
	}

It is the Dashboard Tile/List-Mode-Switch for each Block

[thibsy]

@fhelfer ah, I see. This cannot be fixed using a context renderer then, because the component is already rendered outside of the filter context and passed along as a string. Or am I still missing something here?

[fhelfer]

Ah yes, you're right - this is the wrong example. I think I was trying to solve it in Dashboard and accidentally left this in.

However, it would still be good to have this, as glyphs used inside a Mode-Switch might be a common thing in the future.

[thibsy]

Hi @fhelfer, thx a lot for implementing the changes and for documenting this very precisely. Changes LGTM now, so I will integrate this in all maintained versions.