Update navigateTo for non-custom HTML5 apps #8134
Conversation
| return ContentNodeResource.fetchModel({ id }) | ||
| .then(contentNode => { | ||
| let routeBase, path; | ||
| if (contentNode && contentNode.kind !== 'topic') { |
There was a problem hiding this comment.
Is the contentNode.isLeaf field available from this endpoint? If so, this should also work:
| if (contentNode && contentNode.kind !== 'topic') { | |
| if (contentNode && contentNode.isLeaf) { |
| .then(contentNode => { | ||
| let routeBase, path; | ||
| if (contentNode && contentNode.kind !== 'topic') { | ||
| routeBase = '/topics/c'; |
There was a problem hiding this comment.
I would prefer to a route object here. this route is equivalent to { name: 'TOPICS_CONTENT' m ...}, I think (see the router file in the learn plugin).
| @@ -123,6 +123,9 @@ | |||
| this.hashi.onStateUpdate(data => { | |||
| this.$emit('updateContentState', data); | |||
| }); | |||
| this.hashi.on('navigateto', message => { | |||
| this.$emit('navigateToRegularContext', message); | |||
There was a problem hiding this comment.
I think just navigateTo seems fine here as an event - it's up to the context embedding the content renderer to decide what to do with that.
| // in a custom context, within an overlay, switch the overlay | ||
| // content to the new content | ||
| this.overlayIsOpen = false; | ||
| console.log('new node', contentNode.title); |
| @@ -193,8 +193,19 @@ | |||
| routeBase = '/topics/t'; | |||
There was a problem hiding this comment.
@jonboiser's suggestions about using is_leaf and a route object apply here too.
| } else if (contentNode && this.overlayIsOpen == true) { | ||
| // in a custom context, within an overlay, switch the overlay | ||
| // content to the new content | ||
| this.overlayIsOpen = false; |
There was a problem hiding this comment.
Given that you are not waiting for $nextTick here, I'm not sure that setting false and then true on overlayIsOpen is doing anything. Better off ensuring the overlay renderer is properly reactive to changing its content node, which I think it probably is.
| } | ||
| }) | ||
| .catch(err => { | ||
| console.log(err); |
There was a problem hiding this comment.
Should use the standard API error handling logic here if there's an unexpected error.
marcellamaki
force-pushed
the
update-navigate-to
branch
from
e901e58 to
76a043a
Compare
Summary
This PR creates a function for managing
navigateToevents emitted from HTML5 apps but when they are used outside of a custom navigation context, such as if a user does not have access to these custom contexts but is still able to access the channel. There is a necessary parallel PR to update and KDS to add the corresponding listener onKContentRendererthat is required for this to work. I will add the PR link as a comment once I create the PR.It also updates the
navigateTofunction for custom navigation contexts.Note: The title in the content overlay in the custom channel view isn't updating and thought I've spent some time trying to troubleshoot it, I can't figure out why the value isn't updating. I'm hoping it's obvious that I am just overlooking and someone will have a simple suggestion 😄
Custom Nav Context
"Regular" Context
…
Fixes #8098
…
Reviewer guidance
Before running devserver,
export KOLIBRI_CENTRAL_CONTENT_BASE_URL=https://hotfixes.studio.learningequality.organd download the test channel using token
favim-zinul. If you have already downloaded this channel, you will need to get the most recent changes to the channel, which now include using thenavigateTo()hashi API function within part of the HTML5 app.To test outside a custom context
To test with a custom context
export KOLIBRI_ENABLE_CUSTOM_CHANNEL_NAV=TrueTesting checklist
PR process
Reviewer checklist
yarnandpip)