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

contrib-hotgraphic make popup notify #1525

Closed
chris-steele opened this issue Apr 12, 2017 · 17 comments
Closed

contrib-hotgraphic make popup notify #1525

chris-steele opened this issue Apr 12, 2017 · 17 comments

Comments

@chris-steele
Copy link
Contributor

When accessibility is enabled for larger touch devices, if a hotgraphic popup is left open there is an issue if the user interacts with other components. For example, on an iPad, opening an accordion item results in focus being shifted back to the popup. I can only think the solution here is to go to narrative on all touch devices?

@oliverfoster, @kirsty-hames as a11y disciples, can I get your thoughts on this?

@oliverfoster
Copy link
Member

#1519

@oliverfoster
Copy link
Member

Auto close the popup when clicking away. Drop in a background shadow behind it so the user understands it's a popup.

@kirsty-hames
Copy link
Contributor

I agree @oliverfoster, would be good to handle hotgraphic popups the same way we do notify popups.

@oliverfoster
Copy link
Member

Whilst we're doing this, i think it's worth splitting hotgraphic into two views. A component and its popup.

@chris-steele
Copy link
Contributor Author

@oliverfoster just to confirm, the proposal is to go modal with hotgraphic popups ja?

@oliverfoster
Copy link
Member

i'm not sure we've got consensus on this from anyone in open source yet. so we kinda have two options:

  1. keep the current hotgraphic popup and make that modal
  2. get unanimous support for notify popups from the hotgraphic

@brian-learningpool @dancgray @lc-thomasberger @danielstorey @ryanrolds @canstudios-paulh @moloko ... i've missed loads of people.

@ryanrolds
Copy link
Contributor

I'm in favor of auto-closing if a click/touch event happens outside of the popup (is this #2?). This is not a component that I have used, nor am I doing the work. So, give my opinion less weight.

@canstudios-paulh
Copy link

I'm not sure this post is helpful, but want to let you know you are not talking into a void.
I for one would benefit from knowing where else notify is used? Do other components already us it?
What are the pros and cons? Does it support the same, less or more content? Would it be backward compatible?

@oliverfoster
Copy link
Member

No worries.
Notify is the popup box used for the question feedbacks. It is a module in the core of Adapt which allows us to spring an accessible popup from anywhere else. I looks like this:
image

  • accessible
  • has dismiss behaviour with escape key, close button and click on the background
  • is modal, stops the user from interacting with other content whilst it is open

vs the hotgraphic popup, which looks like this:
image

  • breaks accessibility because it isn't model, all the tab indexes go a bit funny
  • has dismiss behaviour with a close button
  • has back + next controls and an item count

Using Notify instead of the hotgraphic's custom popup has a few issues and solves a few issues:

  • the next, back and item counter would vanish in the immediacy unless a solution can be found that uses the notify and keeps those buttons
  • it'd become accessible
  • it'd allow us to focus on extending the notify behaviour rather than working on a separate popup for hotgraphic

Equally we could make the hotgraphic's pop modal. It'd have a faded background, require the user to complete it before continuing, incorporate the dismiss with escape key, close button and click on background but it would be a shame to duplicate all that behaviour.

@canstudios-paulh
Copy link

Thanks Oliver, that helps a lot.

I agree we shouldn't replicate code if we don't have to. Does notify currently support images (key to the hotspot)?

@oliverfoster
Copy link
Member

Yes it does. But programically it's not great. That, video and more complicated question component style popups are the reasons to concentrate on getting notify up to scratch rather than supporting hotgraphic's popup.

@moloko
Copy link
Contributor

moloko commented Oct 9, 2017

I would be happy for us to switch to Notify if we can keep the next, back and item counter.

It's worth noting that this component says it supports being half-width, but if you do try and do that, this is what you get:
hotgraphic-left
Switching to Notify would therefore make that popup usable if you wanted a half-width hotgraphic.

@chris-steele chris-steele self-assigned this Oct 10, 2017
@chris-steele
Copy link
Contributor Author

chris-steele commented Oct 12, 2017

There is a demo of this in action here. Appreciate not everyone can access and I have tried as I might to provide guest access, but it's not happening. If there is a better place for a public demo please let me know @moloko, @oliverfoster

P.S. I'll update the docs once we're happy.

@oliverfoster
Copy link
Member

@chris-steele try https://rawgit.com/ you can make a repo and upload to git, use that website to make it run like a normal website

it looks beautiful

@oliverfoster
Copy link
Member

@chris-steele is there an open pr for this?

@oliverfoster oliverfoster changed the title contrib-hotgraphic popup style on touch devices contrib-hotgraphic make popup notify Jan 29, 2018
@oliverfoster
Copy link
Member

oliverfoster commented May 22, 2018

closed original pr in favour of adaptlearning/adapt-contrib-hotgraphic#147

@moloko
Copy link
Contributor

moloko commented Jun 26, 2018

hot graphic now switched over to use Notify for popups

@moloko moloko closed this as completed Jun 26, 2018
@moloko moloko added this to the Accessibility v4 milestone Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants