Skip to content

Add alt to the logo, adapt css for logo - #35071

Merged
JuliaKirschenheuter merged 1 commit into
masterfrom
fix/384-missing_alt_text_for_logo_on_multiple_pages
Dec 8, 2022
Merged

Add alt to the logo, adapt css for logo#35071
JuliaKirschenheuter merged 1 commit into
masterfrom
fix/384-missing_alt_text_for_logo_on_multiple_pages

Conversation

@JuliaKirschenheuter

@JuliaKirschenheuterJuliaKirschenheuter commented Nov 10, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

A11y issue regarding alt text for logo on multiple pages

Fix TemplateLayout.php to have an access to $logoUrl outside of the $renderAs === TemplateResponse::RENDER_AS_USER

@JuliaKirschenheuterJuliaKirschenheuter added the 3. to review Waiting for reviews label Nov 10, 2022
@JuliaKirschenheuter

JuliaKirschenheuter commented Nov 10, 2022

Copy link
Copy Markdown
ContributorAuthor

hope i've covered all public / logged in pages
I was not sure about translation: can i leave it like i've done or should i do it different (pleas tell me how)?

@szaimen
szaimen requested review from Pytal and st3inyNovember 10, 2022 10:35
ChristophWurst
ChristophWurst reviewed Nov 11, 2022
View reviewed changes
Comment threadcore/templates/layout.guest.php Outdated
Comment threadcore/css/server.css
@ChristophWurst

ChristophWurst commented Nov 11, 2022

Copy link
Copy Markdown
Member

/backport to stable25

@JuliaKirschenheuterJuliaKirschenheuter mentioned this pull request Nov 11, 2022
Closed
8 tasks
@ChristophWurstChristophWurst added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 11, 2022
@ChristophWurst
ChristophWurst marked this pull request as draft November 11, 2022 09:44
@ChristophWurst

ChristophWurst commented Nov 11, 2022

Copy link
Copy Markdown
Member

In my humble research I found https://css-tricks.com/replace-the-image-in-an-img-with-css/ and https://stackoverflow.com/questions/17705768/is-it-possible-to-change-img-src-attribute-using-css as possible approaches for the css-overwritten logo in the case of theming.

ChristophWurst
ChristophWurst requested changes Nov 11, 2022
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The default logo is no longer replaced but simply overlapped

image

@JuliaKirschenheuter
JuliaKirschenheuterforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from d41d4db to 204bcc3CompareNovember 14, 2022 13:59
@JuliaKirschenheuter
JuliaKirschenheuter marked this pull request as ready for review November 14, 2022 14:00
@JuliaKirschenheuterJuliaKirschenheuter added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 14, 2022
ChristophWurst
ChristophWurst requested changes Nov 16, 2022
View reviewed changes
Comment threadcore/templates/layout.guest.php Outdated
Comment threadcore/templates/layout.public.php Outdated
Comment threadcore/templates/layout.user.php Outdated
@JuliaKirschenheuter
JuliaKirschenheuterforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from 204bcc3 to bcf5895CompareNovember 22, 2022 15:27
@JuliaKirschenheuter
JuliaKirschenheuterforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from bcf5895 to 1f75c3fCompareNovember 23, 2022 14:18
ChristophWurst
ChristophWurst requested changes Nov 23, 2022
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Breaks the logo if there is a custom theme logo

Bildschirmfoto vom 2022-11-23 15-56-03

The image points to /apps/theming/image/logo?useSvg=1&v=47 which returns a 404

@ChristophWurst

ChristophWurst commented Nov 30, 2022

Copy link
Copy Markdown
Member
Undefined index: logoUrl
/drone/src/core/templates/layout.guest.php:38

@ChristophWurstChristophWurst added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Nov 30, 2022
ChristophWurst
ChristophWurst reviewed Nov 30, 2022
View reviewed changes
Comment threadcore/templates/layout.guest.php Outdated
@JuliaKirschenheuter
JuliaKirschenheuterforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from 318c772 to 60a3971CompareNovember 30, 2022 17:14
@JuliaKirschenheuter

JuliaKirschenheuter commented Dec 1, 2022

Copy link
Copy Markdown
ContributorAuthor

/compile amend /

@nextcloud-command
nextcloud-commandforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from 60a3971 to 08deee5CompareDecember 1, 2022 07:34
@JuliaKirschenheuter
JuliaKirschenheuterforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from 08deee5 to 15bad0cCompareDecember 1, 2022 12:43
@JuliaKirschenheuter

JuliaKirschenheuter commented Dec 1, 2022

Copy link
Copy Markdown
ContributorAuthor

/compile amend /

@nextcloud-command
nextcloud-commandforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from 15bad0c to ae33719CompareDecember 1, 2022 16:52
@JuliaKirschenheuterJuliaKirschenheuter added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Dec 2, 2022
@szaimen

szaimen commented Dec 6, 2022

Copy link
Copy Markdown
Contributor

Conflicts...

@JuliaKirschenheuter
Add alt to the logo, adapt css for logo
bba5ab0
Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@JuliaKirschenheuter
JuliaKirschenheuterforce-pushed the fix/384-missing_alt_text_for_logo_on_multiple_pages branch from ae33719 to bba5ab0CompareDecember 8, 2022 08:28
@JuliaKirschenheuter
JuliaKirschenheuter merged commit 11d7cd0 into masterDec 8, 2022
@JuliaKirschenheuter
JuliaKirschenheuter deleted the fix/384-missing_alt_text_for_logo_on_multiple_pages branch December 8, 2022 09:36
@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Dec 8, 2022
Merged
1 task
@danxuliu

danxuliu commented Jan 17, 2023

Copy link
Copy Markdown
Member

This removed the title and, therefore, the name of the shared file in the public share page. Is this known / expected or should I open an issue about it?

Before:
Public-Share-Header-Title-Before

After:
Public-Share-Header-Title-After

@st3iny

st3iny commented Jan 17, 2023

Copy link
Copy Markdown
Member

This removed the title and, therefore, the name of the shared file in the public share page. Is this known / expected or should I open an issue about it?

Before:Public-Share-Header-Title-Before

After:Public-Share-Header-Title-After

Yes, please open a new issue so that it won't get lost.

@skjnldsvskjnldsv mentioned this pull request Jan 17, 2023
Merged
skjnldsv added a commit that referenced this pull request Jan 17, 2023
@skjnldsv
Fix failed revert for#35071
6144693
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv

skjnldsv commented Jan 17, 2023•
edited by nickvergessen
Loading

Copy link
Copy Markdown
Member

Reverted.

nickvergessen added a commit that referenced this pull request Jan 18, 2023
@nickvergessen
Merge pull request#36194from nextcloud/revert/35071
634f5a1
Fix failed revert for #35071
@brunobbrunob mentioned this pull request Jan 18, 2023
Closed
9 tasks
@JuliaKirschenheuterJuliaKirschenheuter mentioned this pull request Feb 6, 2023
Closed
9 tasks
@AndyScherzingerAndyScherzinger mentioned this pull request Feb 16, 2023
Closed
11 tasks
@skjnldsvskjnldsv mentioned this pull request Mar 1, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurstChristophWurstChristophWurst approved these changes
@PytalPytalPytal approved these changes
@CarlSchwanCarlSchwanAwaiting requested review from CarlSchwan
@szaimenszaimenAwaiting requested review from szaimen
@PVince81PVince81Awaiting requested review from PVince81
@st3inyst3inyAwaiting requested review from st3iny

Assignees

@JuliaKirschenheuterJuliaKirschenheuter

Labels

4. to releaseReady to be released and/or waiting for tests to finishaccessibilitybug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@JuliaKirschenheuter@ChristophWurst@szaimen@danxuliu@st3iny@skjnldsv@Pytal