Skip to content

Conversation

@rashadmughal
Copy link

No description provided.

@rashadmughal
Copy link
Author

rashadmughal commented Apr 28, 2021

To ensure the language toggle follows the design guidelines here:

https://design.tax.service.gov.uk/hmrc-design-patterns/welsh-language-toggle/

This what the final output looks like:
image

Final HTML for when English is selected.

<nav class="language-toggle hmrc-language-select" aria-label="Language switcher">
    <ul class="hmrc-language-select__list language-toggle" style="display: block;">
    
        
            <li class="hmrc-language-select__list-item" style="display: inline-block;">
                <span aria-current="true">English</span>
            </li>
        
        
             | 
        
    
        
            <li class="hmrc-language-select__list-item" style="display: inline-block;">
                <a id="cymraeg-switch" href="/customs/register-for-cds/language/cymraeg" hreflang="cy" lang="cy" rel="alternate" target="_self" data-sso="false" data-journey-click="customs-rosm-frontend:language: cy">
                    <span class="visually-hidden">Newid yr iaith ir Gymraeg</span>
                    <span aria-hidden="true">Cymraeg</span>
                </a>
            </li>
        
        
    
    </ul>
</nav>

And for welsh:

<nav class="language-toggle hmrc-language-select" aria-label="Switcher iaith">
    <ul class="hmrc-language-select__list language-toggle" style="display: block;">
    
        
            <li class="hmrc-language-select__list-item" style="display: inline-block;">
                <a id="english-switch" href="/customs/register-for-cds/language/english" hreflang="en" lang="en" rel="alternate" target="_self" data-sso="false" data-journey-click="customs-rosm-frontend:language: en">
                    <span class="visually-hidden">Change language to English</span>
                    <span aria-hidden="true">English</span>
                </a>
            </li>
        
        
             | 
        
    
        
            <li class="hmrc-language-select__list-item" style="display: inline-block;">
                <span aria-current="true">Cymraeg</span>
            </li>
        
        
    
    </ul>
</nav>

@oscarduignan
Copy link
Contributor

Hi @rashadmughal I've taken a bit of a look at this. I'm quite new to the Plat UI team so apologies if my reviews aren't as quick as Jo's might have been here. I need to do a bit more investigation to understand the context of play-ui and where it pulls it's styles from before I can finish reviewing but there are a few small changes I can see would be good to make (just a couple of attributes on links, nothing big) that I'll add as comments while I get myself up to speed properly (just explaining so you're aware there there might be extra feedback after these initial comments)

</a>
</li>
} else {
<li class="hmrc-language-select__list-item" style="display: inline-block;" >
Copy link
Contributor

@oscarduignan oscarduignan Apr 28, 2021

Choose a reason for hiding this comment

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

@rashadmughal I'll see if I can find an alternative for you, but the use of inline style attributes here feels like something we should avoid to prevent unexpected conflicts in the future

@oscarduignan
Copy link
Contributor

@rashadmughal I've discussed this a bit more with other devs from Plat UI team and it looks like the classes you're using here depend on styles from hmrc/hmrc-frontend where as play-ui components can only rely on styles from hmrc/assets-frontend - so to create this component in play-ui it would need to be structured like the language toggle from assets-frontend https://github.com/hmrc/assets-frontend/blob/2692f3349017691e263a64142bde58d84ead4e2e/assets/components/language-selector/language-selector.html

Though we'd strongly recommend that your most future proof option here is to migrate away from play-ui where possible and use hmrc/play-frontend-hmrc which already integrates a language select component https://github.com/hmrc/play-frontend-hmrc/blob/master/src/main/twirl/uk/gov/hmrc/hmrcfrontend/views/components/hmrcLanguageSelect.scala.html

Though we totally understand migration is almost always simpler said than done! So Plat UI is able to help with questions and advice when migrating

@oscarduignan
Copy link
Contributor

oscarduignan commented Apr 28, 2021

Hi @rashadmughal just to add to this, if you do update this PR to use assets-frontend classes to style the component from the example I linked then the other attributes (aria-label, aria-current, lang attributes, and visually hidden copy) shouldn't be removed - the example from assets-frontend doesn't have those bits (but you've got them here) and they are needed for this to meet our current accessibility standards - they're just missing from assets-frontend example because it's not been updated in quite a long time it seems!

@hmrc-web-operations hmrc-web-operations changed the base branch from master to main September 8, 2021 14:07
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