Minor header button improvements

[alexandrevryghem]

References

• Partially resolves larger ticket [Deque Analysis] Header's "serious" accessibility issues #1174
  • one of these was probably for the ExpandableNavbarSectionComponent: 468407, 468408, 468409, 468410
  • Issue ID: 468472

Description

Minor header improvements for header icons spacing (by refactoring the code to use flex gap) & improved search navbar bar accessibility.

Instructions for Reviewers

List of changes in this PR:

• Fixed a html issue: the ExpandableNavbarSectionComponent has a <ul> containing non-<li> elements (here)
• Added flex gap to the navbar button container
• Removed the individual paddings on all the buttons
• Improved the search navbar by preventing it to focus on the input when it's not expanded (Deque issue ID: 468472)
• Themed the LangSwitchComponent for testing purposes in order to check that when the language button is hidden that there is no double padding being applied & that it works for themed components as well

Include guidance for how to test or review your PR:

• Test this by just looking at the header icons in both mobile & desktop mode
• Test it with the dspace theme & the base theme (done by commenting out the dspace theme in src/config/default-app-config.ts)

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes ESLint validation using yarn lint
• My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[alanorth]

Thanks @alexandrevryghem. Tested by applying to my local DSpace 7.6 development environment and updating my own custom theme (based on the dspace theme) to use the new classes and styles. The buttons in the navbar look and behave exactly as before, but in effect have slightly more padding:

If we merge this we need to make sure people know that they will most likely need to update their custom themes.

[alexandrevryghem]

Isn't that always recommended when upgrading your angular version (an alternative is to not just copy the original css file but to extend it instead, this way minor updates like this will always be included). I did however recalculate the pixels today and found out that the search icon in the search bar is also not correctly vertically aligned (see picture I added a background color to make it more visible) so I updated that and I also increased the gap width to match the previous spacing a little bit more.

[alanorth]

(an alternative is to not just copy the original css file but to extend it instead, this way minor updates like this will always be included).

Yeah that's a good idea: instead of copying the SCSS with the component to a theme, importing it instead and overriding the only styles you need. Less duplicated code that goes out of date...

Anyways, I'm +1 on this by testing now. There are tiny differences in the spacing of the buttons, but the functionality is the same. If this addresses the Deque accessibility feedback then I'm in support of merging it.

[tdonohue]

@alexandrevryghem : It appears that e2e tests are failing on this PR because it introduces a new accessibility bug. Here's the details provided by our Axe accessibility scanner (which runs in many e2e tests)

1 accessibility violation was detected
[
  {
    id: 'list',
    impact: 'serious',
    description: 'Ensures that lists are structured correctly',
    helpUrl: 'https://dequeuniversity.com/rules/axe/4.7/list?application=axeAPI',
    nodes: 1,
    html: [
      '<ul _ngcontent-dspace-angular-c185="" class="navbar-nav me-auto mb-2 mb-lg-0 h-100 ng-tns-c185-4">'
    ]
  }
]

[alexandrevryghem]

@tdonohue I indeed forgot to check if the dspace theme still behaved correctly, I fixed the issue so hopefully the CICD should pass now

[tdonohue]

👍 Thanks @alexandrevryghem ! Finally got to testing/reviewing this and it all looks good. I can verify that it resolves several accessibility issues in the header buttons and works well.

[dspace-bot]

Successfully created backport PR for dspace-7_x:

• [Port dspace-7_x] Minor header button improvements #2566