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

New: _canCycleThroughPagination functionality added (fixes #286) #287

Closed
wants to merge 1 commit into from

Conversation

joe-allen-89
Copy link
Contributor

@joe-allen-89 joe-allen-89 commented Dec 8, 2023

#286

New

Potentially put this release on hold until we can discuss/come up with a better way of indicating completion.

@joe-allen-89 joe-allen-89 self-assigned this Dec 8, 2023
@joe-allen-89 joe-allen-89 linked an issue Dec 8, 2023 that may be closed by this pull request
@kirsty-hames
Copy link
Contributor

Potentially put this release on hold until we can discuss/come up with a better way of indicating completion.

Hey @joe-allen-89, I've added some suggestions regarding the display of completion in #288

@kirsty-hames
Copy link
Contributor

Functionality wise, this works as expected testing in Chrome/Safari/FF on Mac and Safari iPhone.

When tabbing/testing with a screen reader, because the back button receives focus before the next button, the back button is read as "Back to Narrative item 3 title (item 3 of 3)". I think this is confusing when you haven't visited the first item yet. From a UX perspective, I'd expect a Narrative to be a linear 1, 2, 3 etc. With this in mind, I think I'm inclined to agree with some of the comments made in the original issue.

Narrative is functionally differrent to the hotgraphic. The main reason cycling is requested for hotgraphic is because you can 'start' at any pin and clients find it too cumbersome to click in and out of pin/notify. Narrative doesn't have this issue as it always starts at the beginning.

@oliverfoster
Copy link
Member

oliverfoster commented Jan 9, 2024

This issue is only really to solve what happens with the _canCycleThroughPagination property when a narrative replaces a hotgraphic on mobile, where the hotgraphic has _canCycleThroughPagination=true.

Narrative did not have its own _canCycleThroughPagination property.
Narrative is always linear, it always starts from index 0 (akin to an ordered list).
Hotgraphic is not linear, the pins can be clicked in any order (akin to an unordered list).
When the narrative replaces the hotgraphic, the list that it represents does not inherit extra order that looping/cycling would break.

In the exploration of this issue we have identified that the narrative does not show completion #288.

Currently, when the narrative replaces a hotgraphic with _canCycleThroughPagination=true, the narrative buttons are not disabled at the ends but they also do not loop/cycle. The buttons are enabled but do nothing #286.

We have two possible choices to solve this issue:

  1. Always disable the narrative buttons at the ends
  2. Allow the narrative buttons to loop/cycle when the narrative replaces a hotgraphic on mobile where the hotgraphic has _canCycleThroughPagination=true

When tabbing/testing with a screen reader, because the back button receives focus before the next button, the back button is read as "Back to Narrative item 3 title (item 3 of 3)".

This is as expected with a looping carousel/narrative according to wcag.

Are you in favour of always disabling the narrative buttons at the ends? @kirsty-hames

I'd like to add _canCycleThroughPagination to the narrative, with your completion fixes and possibly re-add the ability to click the indicators as it is in the wcag implementation.

@kirsty-hames
Copy link
Contributor

Thanks for capturing all the discussion points @oliverfoster.

When tabbing/testing with a screen reader, because the back button receives focus before the next button, the back button is read as "Back to Narrative item 3 title (item 3 of 3)".

This is as expected with a looping carousel/narrative according to wcag.

Are you in favour of always disabling the narrative buttons at the ends? @kirsty-hames

I personally find it odd and would be in favour of always disabling the narrative buttons at the ends. However, regardless of personal opinion, if wcag have set a standard users will be expecting this functionality if it exists elsewhere.

I'd like to add _canCycleThroughPagination to the narrative, with your #288 and possibly re-add the ability to click the indicators as it is in the wcag implementation.

Same goes for re-adding the ability to click the indicators. I personally find the wcag implementation cumbersome to navigate and read the info of each slide. That could be down to my SR keyboard skills though. I'm less keen on adding this but again happy to follow wcag for setting consistent functionality/UI.

Where there can be a lot of difference in opinion when it comes to implementation and this is often questioned by clients. Following wcag does help with those conversations and we have a reference point for following best practice.

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@oliverfoster
Copy link
Member

Following wcag does help with those conversations and we have a reference point for following best practice.

We can choose not to follow.

I agree that the clickable indicators is an odd one. Having them clickable is handy and expected oftentimes for sighted users but terrible for screen reader users. If included it has to be included for both.

The looping behaviour is usually for unordered carousels, having items that could appear in any order. Narrative can also and usually does a linear story with order, where the looping behaviour is a bit less meaningful. Without the completion indication, looping on a mandatory narrative would be a bit unfair, the user would have to remember that they'd seen the first slide or notice the backward slide.

I think the ordered/unordered list distinction is where the contention between wcag and Adapt comes from.

@joe-allen-89
Copy link
Contributor Author

Moving this ticket back to needs reviewing as #301 has been merged allowing us to show completion within the progress indicators.

@guywillis
Copy link
Contributor

Functionally, the proposed works as you'd expect.

However, I agree with Kirsty about the read order being confusing to non visual users. I think I would be in favour of always disabling start and end narrative buttons regardless of the Hotgraphic setting.

@joe-allen-89
Copy link
Contributor Author

After discussion on the OS call most people agree with not including the cycling functionality to the narrative and instead disabling the back/next buttons on last/first items. Moving back to assigned for changes to be made.

@joe-allen-89
Copy link
Contributor Author

Closing PR, as discussed the functionality is changing and a new PR will be raised.

@joe-allen-89 joe-allen-89 deleted the issue/286 branch June 4, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_canCycleThroughPagination conflict with narrative controls
5 participants