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.

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

Display modes and screen reader mode #2950

Merged
merged 4 commits into from
Dec 1, 2024
Merged

Conversation

wofferl
Copy link
Collaborator

@wofferl wofferl commented Nov 30, 2024

Summary

This PR replaces the current display options and adds a screen reader optimized mode

This are the new settings:

display mode: default, compact, screenreader
split mode: vertical, horizontal, off

The screen reader mode uses a single column layout with rendering the original article in the list.

This PR needs to convert the old expand/compactExpand settings:

compact 0 -> displaymode 0, splitmode 0
compact 1, compactExpand 0 -> displaymode 1, splitmode 0
compact 1, compactExpand 1 -> displaymode 1, splitmode 1

@SMillerDev @Grotax where can I find how the conversion should be made?

Checklist

…low combining modes

Signed-off-by: Wolfgang <github@linux-dude.de>
This mode uses the no-spilt mode with rendering the original article per item row. The item row height is set to a fixed value to don't mess up the scroll and for screen readers the item row height doesn't matter

Signed-off-by: Wolfgang <github@linux-dude.de>
@SMillerDev
Copy link
Contributor

Since these are settings and not data, I don't know if it's possible. I also wouldn't mind people having to configure settings one more time. It's not that complicated

…tend use the nextcloud api for loading/saving settings

Signed-off-by: Wolfgang <github@linux-dude.de>
@wofferl
Copy link
Collaborator Author

wofferl commented Nov 30, 2024

Since these are settings and not data, I don't know if it's possible. I also wouldn't mind people having to configure settings one more time. It's not that complicated

sounds good, regarding the unit tests, is /settings still needed. Because the new frontend uses the nextcloud api I think we need new unit tests here.

@SMillerDev
Copy link
Contributor

If it doesn't use that API I think we can remove it.

@wofferl wofferl marked this pull request as ready for review November 30, 2024 15:22
Signed-off-by: Wolfgang <github@linux-dude.de>
@wofferl wofferl merged commit 5e473c1 into nextcloud:master Dec 1, 2024
23 of 24 checks passed
Grotax added a commit that referenced this pull request Dec 1, 2024
Changed
- add region role to content list and details so that it can be used by screen readers (#2946)
- split display settings in "display mode" and "split mode" to allow combining modes (#2950)
- add display mode optimized for screen readers (#2950)

Fixed
- use appropriate semantic HTML elements for the item list to be recognised by screen readers (#2946)
- make title in item list clickable for screen readers to select the current item (#2946)
- remove mime type check when importing `opml`, they are not reliable anyway (#2951)
- Embedded images are all scaled to max (#2945)
- If an article doesn't have title, there's no link to the article (#758)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request Dec 1, 2024
Grotax added a commit that referenced this pull request Dec 1, 2024
Changed
- add region role to content list and details so that it can be used by screen readers (#2946)
- split display settings in "display mode" and "split mode" to allow combining modes (#2950)
- add display mode optimized for screen readers (#2950)

Fixed
- use appropriate semantic HTML elements for the item list to be recognised by screen readers (#2946)
- make title in item list clickable for screen readers to select the current item (#2946)
- remove mime type check when importing `opml`, they are not reliable anyway (#2951)
- Embedded images are all scaled to max (#2945)
- If an article doesn't have title, there's no link to the article (#758)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
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.

Links in item content not clickablee
2 participants