thexerteproject / xerteonlinetoolkits

Xerte Online Toolkits
www.xerte.org.uk
Apache License 2.0
62 stars 61 forks source link

Bootstrap jumping to embedded xot page #1052

Closed ingedonke closed 3 years ago

ingedonke commented 3 years ago

When I add a single page to a bootstrap learning object it sometimes jumps to that embedded page instead of staying on top. See this example: https://xot310beta.dlearning.nl/play.php?template_id=51 I shared the learning object with the demo team in https://xot310beta.dlearning.nl/

Not all pages behave like this by the way. I can't figure out why some pagetypes do this and some don't. For example categories does and flash cards does not.... Can someone take a look?

ronm123 commented 3 years ago

I've had this reported from elsewhere too and the suggestion is that it's something that has changed with 3.10 e.g. happens on existing resources where it didn't happen before the 3.10 upgrade. Also that the page type doesn't matter e.g. happens if the flash card page is the first page too.

It looks like it's an issue with 3.10 xot projects not a problem with Bootstrap. e.g. A 3.9 XOT project embedded in a 3.9 Bootstrap doesn't cause the jumping down to that project. A 3.9 XOT project embedded in a 3.10 Bootstrap doesn't cause the jumping down to that project. A 3.10 XOT project embedded in a 3.9 Bootstrap does cause the jumping down to that project. A 3.10 XOT project embedded in a 3.10 Bootstrap does cause the jumping down to that project.

I'm guessing it's part of the XOT deep linking code in 3.10?

ronm123 commented 3 years ago

@ingedonke @torinfo @FayCross I'm getting reports of this problem from elsewhere now too. Obviously it doesn't stop the bootstrap project loading but the jump is very noticeable and especially bad when the embedded LO is on the first page too. Are you able to replicate this and any idea how to resolve it?

Here's a demo on xot.xerte.org.uk which is a copy of our bootstrap demonstrator with the embedded content page duplicated https://xot.xerte.org.uk/play_281#page1

If you visit #page6 (named embedded content with 3.10 link) either via the menu or directly via https://xot.xerte.org.uk/play_281#page6 you'll see the page jumps down to the embedded LO.

If you visit #page5 (named embedded content with 3.8 link) either via the menu or directly via https://xot.xerte.org.uk/play_281#page5 you'll see the page doesn't jump down to the embedded LO.

I think this suggests the problem happens in Bootstrap projects but is caused by code in 3.10 xot projects but I'm not sure what change caused this or when or more importantly how to resolve it. I think we'd be hearing from more people if everyone was already back in full flow!

FayCross commented 3 years ago

@ronm123 I'll have a look into this now

FayCross commented 3 years ago

@ronm123 it was to do with the x_focusPageContents function that you added in February. I've got it to just ignore that if the project's being shown in an iframe but you might want to check that's ok as I haven't looked at what the purpose of your code was and whether it's still important when the project's embedded

ronm123 commented 3 years ago

@FayCross thanks for looking at this and committing the solution. I've tested and can confirm it resolves the reported issue and reflecting on this it makes sense that the focus on first load of the LO should only happen when the LO itself is the main focus. But I'm guessing (because I haven't checked yet) that the same function is called on page change too which probably means that checking that self == top also stops that focus from happening when a user in the Bootstrap project scrolls down and interacts with the XOT LO. I'll do some testing of that and if true then perhaps just needs another condition adding too?

FayCross commented 3 years ago

@ronm123 - yes, I did wonder whether something else needed to be added to account for when it's embedded but does actually need to be in focus. Let me know if you need a hand with any of this later.

ronm123 commented 3 years ago

@FayCross I've had a quick look and test now and yes we need something else to be added so that it works when using the XOT embedded but I'm not sure what the best solution for that is. Obviously this was part of the accessibility fixes and as per the comment around line 2475 //focus pageContents after page load and page change. So as it stands now with the self == top condition added it works as it did when the LO is top e.g. focus on load and on page change but when embedded the focus now doesn't change on load or on page change.

I guess in pseudo code what we need is if the LO is top or is embedded and has user focus but I don't know how to achieve that - any ideas/suggestions?

torinfo commented 3 years ago

There is a completely different way we could fix this and it would solve another issue as well, but has its own challenges:

When embedding a XOT object in a bootstrap, and the bootstrap is started with LTI/xAPI, all the embedded XOTS are also started with LTI/xAPI. This has the unwelcome side effect that all embedded LO's are tracked/launched even if the user is not actually interacting with them. Kind of a similar situation as with the focus.

So, I thought we could solve this in the same way vimeo/youtube and other streaming services olve this by first presenting an image, and only once clicking the image the actual launch of the LO starts.

Issue is, what image do we present? Ask for it? Generate it?

It would solve the tracking issue and also the focus issue and also would prevent loading all the LO's at the start, but would only load them once the user is actually going to interact with them.

torinfo commented 3 years ago

@FayCross @ronm123 See above...

ronm123 commented 3 years ago

@torinfo @FayCross hmmm it gets more and more complicated doesn't it! :-(

Does the trigger need to be an image? Could it just be a round play button (Transparent SVG image) overlaid in the centre of the LO if embedded and until clicked/activated?

So for the focus code if self == top || activated == true and I guess similar for the LTI code?

Obviously with Vimeo and YouTube etc they automatically grab a thumbnail from the video and also provide options for the author to choose or upload their own but also this is arguably an expected thing with videos but hasn't so far been with embedded Xerte objects. We would probably have to have some kind of optional properties too e.g. if the only way to solve this is a static image representing the LO I would think some authors would choose to have the LO appear as it does now and not worry about screenreader/keyboard users in the embedded version or the fact that tracking is happening without interaction. We might need to create a proof of concept and seek community feedback?

I guess a key question is do we wait until we have discussed this next Thursday or push something to the downloads in the meantime?

torinfo commented 3 years ago

@FayCross @ronm123 Well... transparent overlay could be a solution. It would mean some changes to xenith though, becuase tracking should not start if embedded from bootstrap. Also, if embedded from bootstrap, there will be a get url parameter embed=true.

We should check accessibility as well. But perhaps its a good thing for accessibilty as you can make users aware that this is an embedded LO. Is that worth while to ask someone?

I think we should try to create a POC a.s.a.p. I'll see if I can make some time free for that.

torinfo commented 3 years ago

@FayCross @ronm123 For tracking, the overly will not work if the first page, or the page displayed is an interaction. It will be much to compicated to change the tracking code in the individual models. So for tracking, the LO just should not start at all.

FayCross commented 3 years ago

@ronm123 @torinfo just committed another fix that might be quicker than looking at overlays (probably best for us to discuss that at next dev day?). Not 100% sure this is quite what you are after Ron but is a bit better than previous commit. If not in iframe then pageContents is focused on load and on every page change. If in iframe then pageContents is only focused after page change (so user must be interacting with project at that point)

torinfo commented 3 years ago

@ronm123 @FayCross That should fix the whole issue with the focus I think

Should I push this to master?

I have the whole overlay working as well, and that works fine as well as far as I can tell. But Fays fix gives us time to think this through.

FayCross commented 3 years ago

@ronm123 probably best placed to say whether it's ready for master. Let's talk about the overlay next Thursday

torinfo commented 3 years ago

@FayCross @ronm123 Yes. I agree.

ronm123 commented 3 years ago

@FayCross yes that sounds like a good compromise and big improvement (when in an iframe) to the focus not changing even after page change. Partly this was about a keyboard/screenreader user being able to tab/access content efficiently and when embedded in a page with other contents then obviously the tabbing/focus needs to work with all of the contents on the page not just the embedded LOs. So I haven't tested yet but this seems like it resolves the reported issue whilst still retaining the accessibility enhancement.

@torinfo perhaps this fix should now be pushed to master but wait until the dev day to discuss the overlay? Can you provide a demo of how the overlay works to review before the dev day?

torinfo commented 3 years ago

@ronm123 @FayCross Yes, that was my idea as well. Push Fays fix and show overlay on dev-dy. And yes I have a working installation we can play with. I'll create some examples and will share the links.

torinfo commented 3 years ago

I rolled this out to several clients who complained before, and it seems this has resolved the issue of the jumping around!