Update resources/views/layouts/parts/header-logo.blade.php for accessibility issue

magordon commented

2026-07-01 21:46:09 +02:00

First-time contributor

We had a review by our accessibility team, and overall Bookstack is doing great! However the logo is a bit of an issue for them, since the descriptive alt tag is not descriptive enough as "Logo". Ideally we would like to either extend bookstack to allow the alt tag to be customizable, remove the alt tag and replace with aria-hidden="true", or blank the alt tag so screen readers ignore it. I opted for aria-hidden. I am open to looking into extending the config to allow for it to be configured, but figured I'd try the simplest option first...

Details

We had a review by our accessibility team, and overall Bookstack is doing great! However the logo is a bit of an issue for them, since the descriptive alt tag is not descriptive enough as "Logo". Ideally we would like to either extend bookstack to allow the alt tag to be customizable, remove the alt tag and replace with aria-hidden="true", or blank the alt tag so screen readers ignore it. I opted for aria-hidden. I am open to looking into extending the config to allow for it to be configured, but figured I'd try the simplest option first...

Thank you for all your work on this app!

Checklist

I have read the BookStack community rules.
This PR does not feature significant use of LLM/AI generation as per the community rules above.

We had a review by our accessibility team, and overall Bookstack is doing great! However the logo is a bit of an issue for them, since the descriptive alt tag is not descriptive enough as "Logo". Ideally we would like to either extend bookstack to allow the alt tag to be customizable, remove the alt tag and replace with aria-hidden="true", or blank the alt tag so screen readers ignore it. I opted for aria-hidden. I am open to looking into extending the config to allow for it to be configured, but figured I'd try the simplest option first... ## Details  We had a review by our accessibility team, and overall Bookstack is doing great! However the logo is a bit of an issue for them, since the descriptive alt tag is not descriptive enough as "Logo". Ideally we would like to either extend bookstack to allow the alt tag to be customizable, remove the alt tag and replace with aria-hidden="true", or blank the alt tag so screen readers ignore it. I opted for aria-hidden. I am open to looking into extending the config to allow for it to be configured, but figured I'd try the simplest option first... Thank you for all your work on this app!  ## Checklist  - [x] I have read the [BookStack community rules](https://www.bookstackapp.com/about/community-rules/). - [x] This PR does not feature significant use of LLM/AI generation as per the community rules above.

magordon added 1 commit

2026-07-01 21:46:10 +02:00

Update resources/views/layouts/parts/header-logo.blade.php

analyse-php / build (pull_request) Blocked by required conditions

Details

lint-php / build (pull_request) Blocked by required conditions

Details

test-migrations / build (8.2) (pull_request) Blocked by required conditions

Details

test-migrations / build (8.3) (pull_request) Blocked by required conditions

Details

test-migrations / build (8.4) (pull_request) Blocked by required conditions

Details

test-migrations / build (8.5) (pull_request) Blocked by required conditions

Details

test-php / build (8.2) (pull_request) Blocked by required conditions

Details

test-php / build (8.3) (pull_request) Blocked by required conditions

Details

test-php / build (8.4) (pull_request) Blocked by required conditions

Details

test-php / build (8.5) (pull_request) Blocked by required conditions

Details

49293a8f38

We had a review by accessibility team, and overall Bookstack is doing great, however the logo is a bit of an issue for them, since the descriptive alt tag is not descriptive enough. Ideally we would like to either extend bookstack to allow the alt tag to be customizable, remove the alt tag and replace with aria-hidden="true", or blank the alt tag so screen readers ignore it.

magordon referenced this pull request

2026-07-13 16:27:43 +02:00

Minor accessibilty issue #6171

ps816 referenced this pull request

2026-08-13 14:43:44 +02:00

Fix logo accessibility issue by adding aria-hidden and empty alt tag (#6171) #6183

danb commented

2026-08-27 17:07:17 +02:00

Owner

Hi @magordon,
Thanks for raising the issue and offering this PR.

I've been reading up on best options for accessibility here.
Specifically this webaim page and this blogpost.

Based on what I've read there, simply having a blank alt tag sounds like it's preferred over aria-hidden.

Additionally I think that, if the Show name in header setting (app-name-header setting key) is false, we can assume the logo likely already already aligns, and the user had hidden the text since it's already in the logo. Therefore we could use the setting('app-name') text as alt text when setting('app-name-header') is false.

Hi @magordon, Thanks for raising the issue and offering this PR. I've been reading up on best options for accessibility here. Specifically [this webaim page](https://webaim.org/techniques/alttext/) and [this blogpost](https://www.scottohara.me/blog/2018/05/05/hidden-vs-none.html). Based on what I've read there, simply having a blank alt tag sounds like it's preferred over aria-hidden. Additionally I think that, if the `Show name in header` setting (`app-name-header` setting key) is false, we can assume the logo likely already already aligns, and the user had hidden the text since it's already in the logo. Therefore we could use the `setting('app-name')` text as alt text when `setting('app-name-header')` is false.

analyse-php / build (pull_request) Blocked by required conditions

Details

lint-php / build (pull_request) Blocked by required conditions

Details

test-migrations / build (8.2) (pull_request) Blocked by required conditions

Details

test-migrations / build (8.3) (pull_request) Blocked by required conditions

Details

test-migrations / build (8.4) (pull_request) Blocked by required conditions

Details

test-migrations / build (8.5) (pull_request) Blocked by required conditions

Details

test-php / build (8.2) (pull_request) Blocked by required conditions

Details

test-php / build (8.3) (pull_request) Blocked by required conditions

Details

test-php / build (8.4) (pull_request) Blocked by required conditions

Details

test-php / build (8.5) (pull_request) Blocked by required conditions

Details

This pull request can be merged automatically.

Some workflows are waiting to be reviewed.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u development:magordon-development

git switch magordon-development

Merge

Merge the changes and update on Forgejo.

git switch development

git merge --no-ff magordon-development

git switch magordon-development

git rebase development

git switch development

git merge --ff-only magordon-development

git switch magordon-development

git rebase development

git switch development

git merge --no-ff magordon-development

git switch development

git merge --squash magordon-development

git switch development

git merge --ff-only magordon-development

git switch development

git merge magordon-development

git push origin development

Rows
Columns

Update resources/views/layouts/parts/header-logo.blade.php for accessibility issue #6165

Details

Checklist

Checkout

Merge