Skip to content

Finna 4288 fix content skip link position 2 - #3548

Open
JJManner wants to merge 8 commits into
NatLibFi:devfrom
JJManner:FINNA-4288_fix_content_skip_link_position_2
Open

JJManner wants to merge 8 commits into
NatLibFi:devfrom
JJManner:FINNA-4288_fix_content_skip_link_position_2

Conversation

@JJManner

Copy link
Copy Markdown

This adds - finnaSkipLinkTarget - variable and - record-skip-link-target - to all files that need screenreader a correct - skip to content - focus. Without this fix, screenreaders end up in all kind of irrelevant menus before finally getting to correct - Heading - information. This fix has been tested also in a narrow mobile view.

@JJManner
JJManner requested a review from EreMaijala August 31, 2026 15:47
} elseif ($isLandingPage) {
$contentElementId = 'main';
} elseif ((!empty($this->layout()->finnaSkipLinkTarget))) {
$contentElementId = 'record-skip-link-target';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Miksi tämä on edelleen kovakoodattuna, kun piti käyttää tuota finnaSkipLinkTarget:n arvoa?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tämä edelleen.

@JJManner JJManner Oct 21, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ilman tuota kovakoodattua tietoa ei mikään sivu, esim. - results.phtml - osaa hypätä oikeaan kohtaan, koska - <a class="skip-link" href="#<?=$contentElementId?>"><?=$this->transEsc('Skip to content') // skip to content ?></a> - kohdassa tuo tieto annetaan linkille, jota sitten käytetään kaikissa niissä sivuissa joissa - $this->layout()->finnaSkipLinkTarget = 'record-skip-link-target'; - on määritelty.

@@ -0,0 +1,4 @@
<!-- START of: finna - layout/skiplink-core-heading.phtml -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tämä ei kuulu layout:iin. Laita ennemmin RecordDriver/DefaultRecord/record-title-skiplink.phtml tms.

$iiifManifests = $this->driver->tryMethod('getIiifManifests');

// Add Finna skip link target
$this->layout()->finnaSkipLinkTarget = $finnaSkipLinkTarget = 'record-skip-link-target';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mikä tuon $finnaSkipLinkTarget-muuttujan tarkoitus on?

@JJManner JJManner Sep 22, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Tuo on pakollinen, sillä muuten skiplinkki menee - finnaMainTabs - kohtaan joka ei siis ole suoraan sisältöä.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ei ole. Se on paikallinen muuttuja, jolla ei ole mitään virkaa.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tämä edelleen.


<h1 tabindex="-1" id="results-heading" class="visually-hidden"><?= $this->transEsc('Search Results'); ?></h1>
<h1 tabindex="-1" id="results-heading" class="visually-hidden" aria-hidden="true"><?= $this->transEsc('Search Results'); ?></h1>
<div id="<?=$finnaSkipLinkTarget ?>" role="heading" aria-level="1" class="visually-hidden" ><?= $this->transEsc('Search Results'); ?></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

record-skip-link-target on nyt sitten väärä id hakutuloksille. Id suoraan ilman muuttujaa.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sama juttu kuin ylempänä, muuten ei päädytä sisältöön vaan - searchTabs - osioon.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Edelleen, record-skip-link-target on väärä id hakutuloksissa. Tietuesivulla se on ihan ok. käytettävä id:hän määritellään juuri tuossa templaatin alussa, niin voit siihen laittaa paremman id:n.

@JJManner
JJManner requested a review from EreMaijala September 22, 2026 11:12
@JJManner
JJManner requested a review from EreMaijala October 1, 2026 12:57
} elseif ($isLandingPage) {
$contentElementId = 'main';
} elseif ((!empty($this->layout()->finnaSkipLinkTarget))) {
$contentElementId = $finnaSkipLinkTarget;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mikä muuttuja tuo $finnaSkipLinkTarget on?

$iiifManifests = $this->driver->tryMethod('getIiifManifests');

// Add Finna skip link target
$this->layout()->finnaSkipLinkTarget = $finnaSkipLinkTarget = 'record-skip-link-target';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tämä edelleen.

@JJManner
JJManner requested a review from EreMaijala October 19, 2026 11:32
$this->headLink()->appendAlternate($this->recordLinker()->getActionUrl($this->driver, 'RDF'), 'application/rdf+xml', 'RDF Representation');
}

// Add Finna skip link target

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
// Add Finna skip link target
// Add Finna skip link target

} elseif ($isLandingPage) {
$contentElementId = 'main';
} elseif ((!empty($this->layout()->finnaSkipLinkTarget))) {
$contentElementId = 'record-skip-link-target';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tämä edelleen.

$formats = $this->driver->getFormats();

// Add Finna skip link target
$this->layout()->finnaSkipLinkTarget = $finnaSkipLinkTarget = 'record-skip-link-target';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ja täällä edelleen tuo $finnaSkipLinkTarget-muuttuja..

@JJManner

JJManner commented Oct 21, 2026 •

Copy link
Copy Markdown
Author

EreMaijala
5 hours ago
Member
Tämä edelleen.

Ilman tuota kovakoodattua tietoa ei mikään sivu, esim. - results.phtml - osaa hypätä oikeaan kohtaan, koska - <a class="skip-link" href="#<?=$contentElementId?>"><?=$this->transEsc('Skip to content') // skip to content ?></a> - kohdassa tuo tieto annetaan linkille, jota sitten käytetään kaikissa niissä sivuissa joissa - $this->layout()->finnaSkipLinkTarget = 'record-skip-link-target'; - on määritelty.

@JJManner
JJManner requested a review from EreMaijala October 21, 2026 11:21
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