Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add 'title' attribute to iframe for accessibility #3807

Merged
merged 1 commit into from
Jul 15, 2024
Merged

Add 'title' attribute to iframe for accessibility #3807

merged 1 commit into from
Jul 15, 2024

Conversation

Darshan-upadhyay1110
Copy link
Collaborator

  • Resolves: #
  • Target version: main

Summary

TODO

  • ...

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Documentation (manuals or wiki) has been updated or is not required

@Darshan-upadhyay1110
Copy link
Collaborator Author

Issue:
The absence of a 'title' attribute for an iframe means
that users of screen readers and other assistive technologies will not be able to understand the
content and purpose of the iframe

@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the private/darshan/add-title-attr-in-iframe branch 2 times, most recently from c45cb0f to 13d61cc Compare July 5, 2024 13:37
eszkadev
eszkadev reviewed Jul 5, 2024
View reviewed changes
src/document.js Outdated
@@ -171,7 +171,7 @@ const documentsMain = {
+ '</form>'

// iframe that contains the Collabora Online Viewer
const frame = '<iframe data-cy="coolframe" id="loleafletframe" name="loleafletframe_viewer" allowfullscreen allow="clipboard-read *; clipboard-write *" nonce="' + btoa(getRequestToken()) + '" style="width:100%;height:100%;position:absolute;"/>'
const frame = '<iframe data-cy="coolframe" id="loleafletframe" name="loleafletframe_viewer" allowfullscreen allow="clipboard-read *; clipboard-write *" nonce="' + btoa(getRequestToken()) + '" style="width:100%;height:100%;position:absolute;" title="Collabora Online Viewer"/>'
Copy link
Contributor

Choose a reason for hiding this comment

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

it should be some "PRODUCT NAME" variable string I think

Copy link
Contributor

Choose a reason for hiding this comment

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

@juliushaertl could you help here?

Copy link
Member

Choose a reason for hiding this comment

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

richdocuments/src/view/Office.vue

Line 203 in 612379f

return t('richdocuments', '{productName} is not configured', { productName: loadState('richdocuments', 'productName', 'Nextcloud Office') })
should work here as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@juliushaertl i am not quite sure how things work in .vue files and i am confused how to load product name in [src/document.js]. can you help me here ?

Copy link
Collaborator Author

@Darshan-upadhyay1110 Darshan-upadhyay1110 Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

is it like
<iframe data-cy="coolframe" id="loleafletframe" name="loleafletframe" nonce="' + btoa(getRequestToken()) + '" scrolling="no" allowfullscreen allow="clipboard-read *; clipboard-write *" style="width:100%;height:100%;position:absolute;" title="{productName}"/> ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh i see i think i can import the
import { getCapabilities } from './services/capabilities.ts'

and use getCapabilities().productName as an iframe title

eszkadev
eszkadev reviewed Jul 5, 2024
View reviewed changes
src/document.js Outdated Show resolved Hide resolved
@juliushaertl
Copy link
Member

juliushaertl commented Jul 5, 2024 •
edited
Loading

Also please additionally add this to the iframes in src/view/Office.vue

@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the private/darshan/add-title-attr-in-iframe branch 2 times, most recently from 4f66904 to a495e5d Compare July 9, 2024 07:29
@juliushaertl

This comment was marked as outdated.

Sign in to view
eszkadev
eszkadev reviewed Jul 10, 2024
View reviewed changes
Copy link
Contributor

@eszkadev eszkadev left a comment

Choose a reason for hiding this comment

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

code looks ok now for me

@Darshan-upadhyay1110 @juliushaertl
Add 'title' attribute to iframe for accessibility
f209915 
Signed-off-by: Darshan-upadhyay1110 <darshan.upadhyay@collabora.com>
@juliushaertl juliushaertl force-pushed the private/darshan/add-title-attr-in-iframe branch from a495e5d to f209915 Compare July 15, 2024 05:26
juliushaertl
juliushaertl approved these changes Jul 15, 2024
View reviewed changes
Copy link
Member

@juliushaertl juliushaertl left a comment

Choose a reason for hiding this comment

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

Pushed a small fix for the vue component as there it was not working.

getCapabilities() is no property of the component so it cannot be just used in the template.

@Darshan-upadhyay1110
Copy link
Collaborator Author

Pushed a small fix for the vue component as there it was not working.

getCapabilities() is no property of the component so it cannot be just used in the template.

Thanks @juliushaertl 🙇🏼

@juliushaertl juliushaertl merged commit be6ad4b into main Jul 15, 2024
49 checks passed
@juliushaertl juliushaertl deleted the private/darshan/add-title-attr-in-iframe branch July 15, 2024 07:14
@juliushaertl
Copy link
Member

/backport to stable29

@juliushaertl
Copy link
Member

/backport to stable28

@backportbot backportbot bot mentioned this pull request Jul 15, 2024
Merged
@juliushaertl juliushaertl added the bug Something isn't working label Jul 15, 2024
@backportbot backportbot bot mentioned this pull request Jul 15, 2024
Merged
@github-actions GitHub Actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eszkadev eszkadev eszkadev left review comments

@juliushaertl juliushaertl juliushaertl approved these changes

@pedropintosilva pedropintosilva Awaiting requested review from pedropintosilva

Assignees
No one assigned
Labels
bug Something isn't working feedback-requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Darshan-upadhyay1110 @juliushaertl @eszkadev