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

a11y: entry stream pagination #1135

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

niol
Copy link
Collaborator

@niol niol commented Sep 23, 2019

No description provided.

public/js/selfoss-ui.js Outdated Show resolved Hide resolved
templates/home.phtml Outdated Show resolved Hide resolved
public/js/selfoss-events-entries.js Outdated Show resolved Hide resolved
public/js/selfoss-db.js Outdated Show resolved Hide resolved
public/js/selfoss-ui.js Outdated Show resolved Hide resolved
@jtojnar
Copy link
Member

jtojnar commented Sep 23, 2019

I wonder if using offsets is a good idea with streams. Would not it be safer to use the seek pagination introduced in #869?

@akash07k
Copy link
Contributor

akash07k commented Sep 23, 2019

Implementing the pagination is really a wonderful idea for accessibility as well as usability benefits.
I'm also happy that you used aria-current as well.
Thanks a lot for this. @niol

@niol
Copy link
Collaborator Author

niol commented Sep 23, 2019

I'll take the comments into account later.
What's also missing is navigation hash updates (i.e. https://selfoss.example.com/#unread/all/page-3).

@akash07k
Copy link
Contributor

akash07k commented Sep 23, 2019

I'll take the comments into account later.
What's also missing is navigation hash updates (i.e. https://selfoss.example.com/#unread/all/page-3).

So, are you going to work on that also?

@akash07k
Copy link
Contributor

akash07k commented Sep 23, 2019 via email

@jtojnar
Copy link
Member

jtojnar commented Sep 23, 2019

@akash07k Would it work for you if we showed just previous and next buttons? Are you interested in pagination simply to limit the amount of content on the screen for easier navigation with screen reader, or do you need to be able to switch to arbitrary page?

@niol Could we make this configurable? I actually prefer to be able to scroll back to the previous items. Also, how does this interact with auto_stream_more option?

@akash07k
Copy link
Contributor

akash07k commented Sep 23, 2019

@akash07k Would it work for you if we showed just previous and next buttons? Are you interested in pagination simply to limit the amount of content on the screen for easier navigation with screen reader, or do you need to be able to switch to arbitrary page?

@niol Could we make this configurable? I actually prefer to be able to scroll back to the previous items. Also, how does this interact with auto_stream_more option?

@akash07k Would it work for you if we showed just previous and next buttons? Are you interested in pagination simply to limit the amount of content on the screen for easier navigation with screen reader, or do you need to be able to switch to arbitrary page?

@niol Could we make this configurable? I actually prefer to be able to scroll back to the previous items. Also, how does this interact with auto_stream_more option?

Actually, it would be very good and sofisticated if the navigation for the arbitrary pages will be provided by the pagination impletation.
But if it is very difficult to achieve and implement, then we can just compromise with 'next' and 'previous' buttons only.

@jtojnar
Copy link
Member

jtojnar commented Sep 23, 2019

But if it is very difficult to achieve and implement, then we can just compromise with 'next' and 'previous' buttons only.

It should be possible to combine seek pagination with explicit pages (via seek+offset) but the question is if we should. Selfoss strives to be a simple program to use so we need to always ask if the feature would help someone achieve a concrete existing goal. Hypothetical use cases or the fact that other software provides the feature is not sufficient reason to implement it.

I cannot come up with a task that would jumping to a specific page achieve, so I want to know more about your use case. For example, for the “Next” button, we could say: “I have read all the news on this page and want to read more.”

Also if we wanted to keep the page numbers, we would need to trim the navigation somewhat to avoid too many buttons:

Screenshot of the items view in selfoss with this patch applied, scrolled to the bottom of the page, the 289 buttons for switching page occupying most of the screen

Having more information about your use case would allow us to make the pagination usable for you, while omitting everything that you do not need. I assume having screen reader to navigagate 289 buttons will not be pleasant either.

@akash07k
Copy link
Contributor

akash07k commented Sep 23, 2019

But if it is very difficult to achieve and implement, then we can just compromise with 'next' and 'previous' buttons only.

It should be possible to combine seek pagination with explicit pages (via seek+offset) but the question is if we should. Selfoss strives to be a simple program to use so we need to always ask if the feature would help someone achieve a concrete existing goal. Hypothetical use cases or the fact that other software provides the feature is not sufficient reason to implement it.

I cannot come up with a task that would jumping to a specific page achieve, so I want to know more about your use case. For example, for the “Next” button, we could say: “I have read all the news on this page and want to read more.”

Also if we wanted to keep the page numbers, we would need to trim the navigation somewhat to avoid too many buttons:

Screenshot of the items view in selfoss with this patch applied, scrolled to the bottom of the page, the 289 buttons for switching page occupying most of the screen

Having more information about your use case would allow us to make the pagination usable for you, while omitting everything that you do not need. I assume having screen reader to navigagate 289 buttons will not be pleasant either.

Actually, if it is too dificult then we can only have just previous and next buttons only.
We can only have 4-5 pagination links on the page and then we can have the links for first page and last page entirely along with the next and previous page links.
Also, it can be needed in the cases, for example let's say 50 items are displaying on a page and we directly want to read the news from the past let's say from 4th page, then in this case it will help.
But again, if it is difficult and complicated, then we can only have just next and previous buttons/links.
Also, there's no need to show all the pagination numbers.

@niol
Copy link
Collaborator Author

niol commented Sep 24, 2019

There is no technical difficulty but I won't use this so cannot input on the use case. I'll follow-up on the navigation hash and configurability when what's wanted is more clear.

@akash07k
Copy link
Contributor

There is no technical difficulty but I won't use this so cannot input on the use case. I'll follow-up on the navigation hash and configurability when what's wanted is more clear.

So, what will be the current implementation of the paginations as of now?

@niol
Copy link
Collaborator Author

niol commented Sep 24, 2019

What's proposed here is adding page 1 to 5 buttons with current page indication. It works but needs ironing out.

@akash07k
Copy link
Contributor

What's proposed here is adding page 1 to 5 buttons with current page indication. It works but needs ironing out.

Thanks, it's good. also, it will also be good if there will be pagination for the first and last page as well. BTW, thanks a tons for your effort bro. Till when can you complete it fully?

@akash07k
Copy link
Contributor

akash07k commented Oct 3, 2019

Is it ready for merge?

@niol
Copy link
Collaborator Author

niol commented Oct 3, 2019

I wonder if using offsets is a good idea with streams. Would not it be safer to use the seek pagination introduced in #869?

In order to seek, you need to know the last item of each page to be able to jump. offset is the only way.

@niol
Copy link
Collaborator Author

niol commented Oct 3, 2019

Is it ready for merge?

No. Missing:

  • navigation hash updates
  • see if this can be configurable
  • add links to first and last pages
  • rebasing

@akash07k
Copy link
Contributor

akash07k commented Oct 3, 2019 via email

@akash07k
Copy link
Contributor

Oh, ok. :(

On 10/3/19, niol @.***> wrote: > > > Is it ready for merge? No. Missing: - navigation hash updates - see of this can be configurable - add links to first and last pages - rebasing -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #1135 (comment)

@jtojnar
@niol Guys, can you please complete this implementation soon?
We need it because infinite scrolling cause lots of problems and after a lot of entries gets loaded the page starts to hang due to the heaviness.
I'm just requesting, please don't feel bad, this is just asimple request. :(

@niol
Copy link
Collaborator Author

niol commented Oct 20, 2019

I'm still not satisfied with what I have and I'm lacking time right now to look into it again.

  • navigation hash updates: not sure it is worth it
  • see if this can be configurable: not sure if worth it to introduce new parameter
  • add links to first and last pages: done
  • rebasing: I need to learn a whole new client build process and caveats, so may take some time

@akash07k
Copy link
Contributor

I'm still not satisfied with what I have and I'm lacking time right now to look into it again.

  • navigation hash updates: not sure it is worth it
    In my opinion it will be worth it, rest @jtojnar your view on this?
  • see if this can be configurable: not sure if worth it to introduce new parameter
  • add links to first and last pages: done
  • rebasing: I need to learn a whole new client build process and caveats, so may take some time
    @jtojnar
    Can you please help?

@jtojnar
Copy link
Member

jtojnar commented Oct 21, 2019

Unfortunately, I am also busy. But feel free to hit me with any questions about the client changes.

@akash07k
Copy link
Contributor

@niol
@jtojnar
Is there any progress/update on this?

@jtojnar
Copy link
Member

jtojnar commented Dec 22, 2019

I will be busy until February.

@akash07k
Copy link
Contributor

akash07k commented Dec 22, 2019 via email

@akash07k
Copy link
Contributor

akash07k commented May 1, 2020

Hey @niol ,
Any updates buddy?

@jtojnar jtojnar mentioned this pull request Sep 14, 2020
@jtojnar jtojnar added this to the 2.20 milestone Oct 14, 2022
@jtojnar jtojnar removed this from the 2.20 milestone Mar 29, 2023
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.

3 participants