Skip to content

Commit

Permalink
ucfopen#1948 Adds an 'ignore lock' toggle to nav:goto actions which d…
Browse files Browse the repository at this point in the history
…efaults to true, allowing all existing button-based navigation to continue working while also allowing authors to respect navlocks, or at least inform them when they do not.
  • Loading branch information
FrenjaminBanklin committed Jan 21, 2022
1 parent 876b13c commit 37392cd
Show file tree
Hide file tree
Showing 10 changed files with 194 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ Array [
]
`;

exports[`NavStore nav:goto does not change page when locked 1`] = `undefined`;
exports[`NavStore nav:goto does not change page when locked if ignoreLock is false 1`] = `undefined`;

exports[`NavStore nav:gotoPath event calls gotoItem and postEvent 1`] = `
Array [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,17 +366,94 @@ describe('NavStore', () => {
expect(Dispatcher.trigger).not.toHaveBeenCalled()
})

test('nav:goto does not change page when locked', () => {
test('nav:goto does nothing if payload is undefined', () => {
NavStore.setState({
isInitialized: true,
navTargetId: 7,
navTargetId: 'mockId',
itemsById: {
mockId: { id: 'mockId', flags: {} }
}
})

expect(Dispatcher.trigger).not.toHaveBeenCalled()
eventCallbacks['nav:goto']()
expect(Dispatcher.trigger).not.toHaveBeenCalled()
})

test('nav:goto does nothing if payload.value is undefined', () => {
NavStore.setState({
isInitialized: true,
navTargetId: 'mockId',
itemsById: {
mockId: { id: 'mockId', flags: {} }
}
})

expect(Dispatcher.trigger).not.toHaveBeenCalled()
eventCallbacks['nav:goto']({})
expect(Dispatcher.trigger).not.toHaveBeenCalled()
})

test('nav:goto still changes page when locked if ignoreLock is not provided', () => {
NavStore.setState({
isInitialized: true,
navTargetId: 'mockId',
navTargetHistory: [],
itemsById: {
mockId: { id: 'mockId', flags: {} }
},
locked: true
})

const spy = jest.spyOn(NavStore, 'gotoItem')
NavStore.gotoItem.mockReturnValueOnce(true)

// go
eventCallbacks['nav:goto']()
eventCallbacks['nav:goto']({ value: { id: 'mockId' } })
expect(NavStore.gotoItem).toHaveBeenCalledTimes(1)
expect(NavStore.gotoItem).toHaveBeenCalledWith({ flags: {}, id: 'mockId' })
expect(ViewerAPI.postEvent).toHaveBeenCalledTimes(1)

spy.mockRestore()
})

test('nav:goto still changes page when locked if ignoreLock is true', () => {
NavStore.setState({
isInitialized: true,
navTargetId: 'mockId',
navTargetHistory: [],
itemsById: {
mockId: { id: 'mockId', flags: {} }
},
locked: true
})

const spy = jest.spyOn(NavStore, 'gotoItem')
NavStore.gotoItem.mockReturnValueOnce(true)

// go
eventCallbacks['nav:goto']({ value: { id: 'mockId', ignoreLock: true } })
expect(NavStore.gotoItem).toHaveBeenCalledTimes(1)
expect(NavStore.gotoItem).toHaveBeenCalledWith({ flags: {}, id: 'mockId' })
expect(ViewerAPI.postEvent).toHaveBeenCalledTimes(1)

spy.mockRestore()
})

test('nav:goto does not change page when locked if ignoreLock is false', () => {
NavStore.setState({
isInitialized: true,
navTargetId: 'mockId',
itemsById: {
mockId: { id: 'mockId', flags: {} }
},
locked: true
})

const spy = jest.spyOn(NavStore, 'gotoItem')

// go
eventCallbacks['nav:goto']({ value: { id: 'mockId', ignoreLock: false } })
expect(NavStore.gotoItem).not.toHaveBeenCalled()
expect(ViewerAPI.postEvent).not.toHaveBeenCalled()
expect(ViewerAPI.postEvent.mock.calls[0]).toMatchSnapshot()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,26 @@ describe('NavUtil', () => {
expect(x).toBe('mockTriggerReturn')
})

test('goto', () => {
test('goto (ignoreLock not provided)', () => {
expect(Common.flux.Dispatcher.trigger).not.toHaveBeenCalled()
const x = NavUtil.goto('mockId')
const expectedValue = {
value: {
id: 'mockId'
id: 'mockId',
ignoreLock: true
}
}
expect(Common.flux.Dispatcher.trigger).toHaveBeenCalledWith('nav:goto', expectedValue)
expect(x).toBe('mockTriggerReturn')
})

test('goto (ignoreLock provided)', () => {
expect(Common.flux.Dispatcher.trigger).not.toHaveBeenCalled()
const x = NavUtil.goto('mockId', false)
const expectedValue = {
value: {
id: 'mockId',
ignoreLock: false
}
}
expect(Common.flux.Dispatcher.trigger).toHaveBeenCalledWith('nav:goto', expectedValue)
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,14 @@ describe('TriggerListModal', () => {
// change the value
component
.find('input')
.at(2)
.at(3)
.simulate('change', { target: { type: 'text', value: '10' } })

// check that the value changed
expect(
component
.find('input')
.at(2)
.at(3)
.props()
).toHaveProperty('value', '10')

Expand All @@ -270,6 +270,54 @@ describe('TriggerListModal', () => {
expect(tree).toMatchSnapshot()
})

test('sets value that was previously undefined', () => {
const content = {
triggers: [
{
type: 'onMount',
actions: [{ type: 'nav:goto', value: { id: 1 } }]
},
{
type: 'onUnmount',
actions: []
}
]
}
const component = mount(<TriggerListModal content={content} />)

// make sure this is the expected label/input combo
const inputLabel = component.find('label').at(2)
expect(inputLabel.props().children).toBe('Item Id')

expect(
component
.find('input')
.at(2)
.props()
).toHaveProperty('checked', true)

// change the value
component
.find('input')
.at(2)
.simulate('change', { target: { type: 'boolean', value: false } })

// check that the value changed
expect(
component
.find('input')
.at(2)
.props()
).toHaveProperty('checked', false)

// check the change to state
expect(component.state()).toHaveProperty('triggers')
expect(component.state().triggers[0].actions).toContainEqual({
type: 'nav:goto',
value: { id: 1, ignoreLock: false }
})
})

test('changes scroll type', () => {
const content = {
triggers: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ class TriggerListModal extends React.Component {
createNewDefaultActionValueObject(type) {
switch (type) {
case 'nav:goto':
return {
id: '',
ignoreLock: true
}

case 'assessment:startAttempt':
case 'assessment:endAttempt':
return {
Expand Down Expand Up @@ -230,6 +235,17 @@ class TriggerListModal extends React.Component {
value={action.value.id || ''}
onChange={this.updateActionValue.bind(this, triggerIndex, actionIndex, 'id')}
/>
<Switch
title="Ignore Navigation Lock"
// eslint-disable-next-line no-undefined
checked={action.value.ignoreLock === undefined ? true : action.value.ignoreLock}
onChange={this.updateActionValue.bind(
this,
triggerIndex,
actionIndex,
'ignoreLock'
)}
/>
</div>
</div>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,19 @@ class NavStore extends Store {
}
},
'nav:goto': payload => {
/* eslint-disable no-undefined */
if (
payload === undefined ||
payload.value === undefined ||
payload.value.id === undefined
) {
return
}
if (payload.value.ignoreLock === undefined) payload.value.ignoreLock = true
/* eslint-enable no-undefined */

if (this.state.locked && !payload.value.ignoreLock) return

if (!this.state.isInitialized) {
this.pendingTarget = {
type: 'goto',
Expand All @@ -135,8 +148,6 @@ class NavStore extends Store {
return
}

if (this.state.locked) return

oldNavTargetId = this.state.navTargetId
const navItem = this.state.itemsById[payload.value.id]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,13 @@ const NavUtil = {
return Dispatcher.trigger('nav:next')
},

goto(id) {
goto(id, ignoreLock) {
// eslint-disable-next-line no-undefined
if (ignoreLock === undefined) ignoreLock = true
return Dispatcher.trigger('nav:goto', {
value: {
id
id,
ignoreLock
}
})
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ const ActionButtonEditorAction = props => {
} else {
description = 'Go to ""'
}
// eslint-disable-next-line no-undefined
if (props.value.id && props.value.ignoreLock === undefined ? true : props.value.ignoreLock) {
description = `${description} (Ignore Navigation Lock)`
}

break
case 'nav:prev':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ test('Action Button Editor displays id for nav:goto trigger', () => {
const value = { id: 'mockId' }

const component = mount(<ActionButtonEditorAction type={type} value={value} />)
expect(component.find('span').html()).toEqual('<span>Go to mockId</span>')
expect(component.find('span').html()).toEqual(
'<span>Go to mockId (Ignore Navigation Lock)</span>'
)
})

test('Action Button Editor displays empty id for nav:goto trigger', () => {
Expand Down

0 comments on commit 37392cd

Please sign in to comment.