chore: add on-event behavior to allowed list for navigation behaviors #734
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reduced complexity of determining the url
The original implementation for handling `<navigator>` elements nested inside `<nav-route>` elements relied on storing Dom elements in a map by the id of the route which contained them. This had several problems: - A cache had to be created - The items in the cache could become orphaned from their parent doc - Mutations to the doc would not be reflected in the cache - The cache was never cleared - The id or index of the implementation could change This new solution uses a context to wrap every `hv-route` which contains its own document. This allows child routes to retrieve their Dom element and retrieve any nested navigator direction from the doc. Improvements are also made to the initial route by creating all routes into the stack navigator.
… the existing doc (#618) As the first step in deep link support, this update supports full merging of the existing dom with an incoming deep link document. The structure of the incoming document remains the same as any other \<navigator\> document structure with the following exceptions: - For any \<navigator\> node, the addition of an attribute `merge="true"` will enable merging the navigator children. Omitting the attribute will cause any \<navigator\> to replace an existing one of the same name - Merging will add any child \<nav-route\> nodes which don't exist and will update those which do - All `selected` values from the current document are reset to "false" to allow the incoming document to set a new selected path In the example below, the deep link provides the following changes: 1. The shifts-route receives a sub-navigator 2. The tabs-navigator receives an additional 'settings' tab 3. The root-navigator receives a new 'account-popup' route 4. The initial route is now `root-navigator->tabs-route->tabs-navigator->shifts->shifts-route->past-shifts` Because all incoming data indicated `merge="true"`, all of the changes were additive. Original: ```xml <doc xmlns="https://hyperview.org/hyperview"> <navigator id="root-navigator" type="stack"> <nav-route id="tabs-route"> <navigator id="tabs-navigator" type="tab"> <nav-route id="live-shifts-route" href="/biz-app-hub" selected="true" /> <nav-route id="shifts-route" href="/biz-app-shift-group-list" /> <nav-route id="account-route" href="/biz-app-account" /> </navigator> </nav-route> </navigator> </doc> ``` Deep link: ```xml <doc xmlns="https://hyperview.org/hyperview"> <navigator id="root-navigator" type="stack" merge="true"> <nav-route id="tabs-route"> <navigator id="tabs-navigator" type="tab" merge="true"> <nav-route id="shifts-route" href="" selected="true"> <navigator id="shifts" type="tab" merge="true"> <nav-route id="upcoming-shifts" href="biz-app-shift-group-list" selected="false" /> <nav-route id="past-shifts" href="/biz-app-shift-group-list'" selected="true" /> </navigator> </nav-route> <nav-route id="settings" href="/biz-app-account" selected="false" /> </navigator> </nav-route> <nav-route id="account-popup" href="/biz-app-account" modal="true" /> </navigator> </doc> ``` Merged: ```xml <doc xmlns="https://hyperview.org/hyperview"> <navigator id="root-navigator" type="stack" merge="true"> <nav-route id="tabs-route" selected="false"> <navigator id="tabs-navigator" type="tab" merge="true"> <nav-route id="live-shifts-route" href="/biz_app/gigs/live_shifts" selected="false" /> <nav-route id="shifts-route" href="/biz_app/gigs/groups" selected="true"> <navigator id="shifts" type="tab" merge="true"> <nav-route id="upcoming-shifts" href="/biz_app/gigs/groups" selected="false" /> <nav-route id="past-shifts" href="/biz_app/gigs/groups" selected="true" /> </navigator> </nav-route> <nav-route id="account-route" href="/biz_app/account" selected="false" /> <nav-route id="settings" href="/biz_app/account" selected="false" /> </navigator> </nav-route> <nav-route id="account-popup" href="/biz_app/account" modal="true" /> </navigator> </doc> ```
- Moving the route search into helpers - Moving the local types into types.ts
…ute (#623) Reflect the user's selections in the document - Use the 'focus' listener to allow each hv-route to set its selected state within the document - De-select any sibling routes to ensure the current is the only selected Note: I did not use 'blur' to have each route deselect itself as that would cause routes in the background to lose their state. In the video, as the user selects between the different tabs, the various \<nav-route\> children of `tabs-navigator` are shown to reflect current selection. When the `company` modal is popped up, the `company` \<nav-route\> shows selected while its sibling `tabs-route` becomes deselected. Note that the `tabs-navigator` children retain their correct selection state. Asana: https://app.asana.com/0/0/1205197138955014/f https://github.com/Instawork/hyperview/assets/127122858/0687558f-c500-419a-bd69-bbd0843ade72
…vigators (#624) Previous implementation used 'dynamic' and 'modal' routes as the only two children of `stack` navigators. The urls were cached into a context and retrieved by id. This update allows each defined `\<nav-route\>` to be added to its navigator. These known routes can retain their original url and presentation and reduce the complexity of navigating between them. One note about the implementation: the 'initialParams' passed into each route contain their actual url. When we navigate to a route through a behavior, the new params are merged with the existing. The url had to explicitly be deleted from the object as passing a url:undefined would overwrite the initial url value with an 'undefined' value. Asana: https://app.asana.com/0/0/1205197138955012/f
When an <hv-route> is initially defined to load its own data via url, it
will store its <doc> in state and will pass it down to its children.
When a deep link is processed a provides additional information for the
route, the route must abandon its local state.doc and rely on the
element passed in from its parent.
Initial doc:
``` xmldoc
<doc ...>
<navigator id="tabs-navigator" type="tab">
<nav-route id="shifts-route" href="{% url 'biz-app-sub-navigator' %}" selected="true"/>
</navigator>
</doc>
```
The `<hv-route>` representing `shifts-route` will have loaded its own
`<doc/>` and will have stored it in its state.
Deep link:
``` xmldoc
<doc ...>
<navigator id="tabs-navigator" type="tab">
<nav-route id="shifts-route" href="" selected="true">
<navigator id="shifts" type="tab" merge="true">
<nav-route id="upcoming-shifts" href="{% url 'biz-app-shift-group-list' %}" selected="true"/>
<nav-route id="past-shifts" href="{% url 'biz-app-shift-group-list' %}" selected="false"/>
</navigator>
</nav-route>
</navigator>
</doc>
```
The deep link has provided additional context for `shifts-route` by now
providing a sub-navigator `shifts` and the relevant routes.
The `tabs-navigator` remains unchanged; it still has one route
`shifts-route`, which means the `<hv-route>` representing `shifts-route`
is not unmounted. However, that route contains a `<doc>` in state which
is now invalid.
This code change allows the `<hv-route>` to prioritize data received
from its parent over any doc it has in state.
Asana: https://app.asana.com/0/0/1205197143810570/f
Using the `cardStyleInterpolator` option to force iOS to render modals at full screen while retaining their intended animation. | Default | Updated iOS | Updated Android | | ------ | ----- | ----- | |  |  |  | Asana: https://app.asana.com/0/0/1205197143810575/f
Changed approach to use `getDerivedStateFromProps` to drive the state.doc value
- use the more performant `getFirstChildTag` since all these locations know their structure - simplified the usage of the internal component to not require its own props type
…640) A few code refactors which will help the next step: 1. Created a helper to determine if a route id is one of the generated 'dynamic/modal' routes 2. Centralized the `screenOptions` for stack navigators to reduce redundant code 3. Created a better `getId` function for getting the id of dynamic and defined routes 4. Moved the creation of dynamic routes into its own function 5. Re-use `buildScreen` when building dynamic routes to avoid redundant code
When a modal is opened, there may be additional navigation which happens within its content. If the modal implements its own stack, those navigation actions can occur within the view of the modal. They can be closed without having to back out of all the screens. In the following videos, the 'company' view is defined as a modal. Without the stack navigator, the 'edit' action replaces the 'company' view and the user has to go back before closing the modal. With the stack navigator, the user can close the modal at any point. Note: default react-navigation UI is being used in these videos to simplify the navigation. | No stack | With stack | | ---- | ---- | |  |  | Asana: https://app.asana.com/0/1204008699308084/1205225573228523/f
Replaced all uses of 'dynamic' as a type of route to 'card' to better reflect the implementation details.
Remove instances of
`const { someFunction } = this`
and
`const { someProp } = props`
The initial document may contain multiple <nav-route> elements in a stack. These will be pushed onto the stack immediately. As they are closed they should be removed from the dom to reflect the current state. Note: card and modal routes pushed onto the stack by user activity will not be added to the dom to reflect state. Asana: https://app.asana.com/0/0/1205225573228534/f --- Example: closing the 'company' modal. Before: ```XML <doc ...> <navigator id="root-navigator" type="stack"> <nav-route id="tabs-route" selected="false"> <navigator id="tabs-navigator" type="tab"> <nav-route id="live-shifts-route" href="/biz_app/gigs/live_shifts" selected="false"/> <nav-route id="shifts-route" href="/biz_app/sub-navigator" selected="true"/> <nav-route id="account-route" href="/biz_app/account" selected="false"/> </navigator> </nav-route> <nav-route id="company" href="/biz_app/account/company" modal="true" selected="true"/> </navigator> </doc> ``` After: ```XML <doc ...> <navigator id="root-navigator" type="stack"> <nav-route id="tabs-route" selected="false"> <navigator id="tabs-navigator" type="tab"> <nav-route id="live-shifts-route" href="/biz_app/gigs/live_shifts" selected="false"/> <nav-route id="shifts-route" href="/biz_app/sub-navigator" selected="true"/> <nav-route id="account-route" href="/biz_app/account" selected="false"/> </navigator> </nav-route> </navigator> </doc> ```
Making the tab navigator setup more closely resemble the stack navigator
No need to pass separate props object into the components
Using constants for consistency
- Merged the required new functionality from `route-doc` into the newly added `DocContext` - Updated the implementation of HvRoute to provide the getter and setter - Cleaned up usage and naming of the value --------- Co-authored-by: Florent Bonomo <flochtililoch+github@gmail.com>
This is a working prototype of deep links with custom navigators. - For stack navigators, the custom navigator provides an injection of a custom router which overrides the behavior of initial state and updates for new routes - In the override, all routes are pushed onto the stack - For tab navigators, the custom navigator calls a 'navigate' whenever the initial route name prop changes This functionality requires mobile branch https://github.com/Instawork/mobile/tree/hardin/deep-link-url-wip **Tabs** With the default behavior, the initial state is correct: shifts-route (middle tab), and second-shifts (right sub tab). However, the deep link has no effect. The current tab selection remains. Deep link ```xml <doc {% namespaces %}> <navigator id="root-navigator" type="stack" merge="true"> <nav-route id="tabs-route"> <navigator id="tabs-navigator" type="tab" merge="true"> <nav-route id="shifts-route" href="{% url 'biz-app-sub-navigator' %}" selected="true"> <navigator id="shifts" type="tab" merge="true"> <nav-route id="second-shifts" href="{% url 'biz-app-shift-group-list' %}" selected="true"/> </navigator> </nav-route> </navigator> </nav-route> </navigator> </doc> ``` | initial state | deep link | | -- | -- | |  |  | With an override navigator, the deep link is able to change the selection in both the main tab and the sub-tab navigators. | initial state | deep link | | -- | -- | |  |  | **Stacks** With the default behavior, the stack navigator pushes only one screen onto the stack: the one with a naming matching the "initialRouteName" property value. Deep links provide no additional functionality. Initial state (index.xml) ```xml <doc {% namespaces %}> <navigator id="root-navigator" type="stack"> <nav-route id="tabs-route"> <navigator id="tabs-navigator" type="tab"> <nav-route id="live-shifts-route" href="{% url 'biz-app-hub' %}"/> <nav-route id="shifts-route" href="{% url 'biz-app-sub-navigator' %}" selected="true"/> <nav-route id="account-route" href="{% url 'biz-app-account' %}" selected="true"/> </navigator> </nav-route> <nav-route id="company" href="{% url 'biz-app-account-company' %}" modal="true"/> </navigator> </doc> ``` Deep link ```xml <doc {% namespaces %}> <navigator id="root-navigator" type="stack" merge="true"> <nav-route id="account" href="{% url 'biz-app-account' %}" modal="true"/> </navigator> </doc> ``` Given this initial state, the desired outcome is to have the "company" modal on top of the tabs. | initial state | deep link | | -- | -- | |  |  | After overriding the navigator and router, the stack is able to push all screens onto the stack in the initial state and is able to push screens onto the stack when a deep link is received. NOTE: There is currently a bug causing the stack initial state to open and immediately close the modal screen. It is caused by the overrides of tabs and stack working against each other. A separate task will be created to look into fixes. | initial state | deep link | | -- | -- | |  |  | Asana: https://app.asana.com/0/1204008699308084/1205129681586820/f
Fix for issue where a tab navigation is auto closing any stack modals. Asana: https://app.asana.com/0/1204008699308084/1205572796301526/f
# Conflicts: # src/components/hv-list/index.tsx # src/components/hv-section-list/index.tsx # src/contexts/index.ts # src/contexts/navigator-map.tsx # src/core/components/hv-navigator/index.tsx # src/core/components/hv-navigator/types.ts # src/core/components/hv-route/index.tsx # src/core/components/hv-route/types.ts # src/services/dom/helpers-legacy.ts # src/services/navigator/helpers.test.ts # src/services/navigator/helpers.ts # src/types-legacy.ts
Resolving CI issues found after merging master with TS migration into integration branch
Moving the behavior trigger code out of `hyper-ref` into a shared location to allow access by other components Asana: https://app.asana.com/0/1204008699308084/1205741965789631/f
…root (#727) Follow the commits for the individual steps taken - moved the 'once' handling methods into /services/behaviors - updated `BehaviorOptions` to properly reflect the state of the data - created new `RootOnUpdate` and `OnUpdateCallbacks` types - added `onUpdate` to all props which occur from `hv-root` to `hv-screen` - ported the `onUpdate` method and all of its related methods from `hv-screen` to `hv-root` - use the callbacks prop to inject any of the instance-specific callbacks needed for the `onUpdate` - inject the new `onUpdate` into the props Asana: https://app.asana.com/0/1204008699308084/1205741965789634/f --------- Co-authored-by: Florent Bonomo <flochtililoch+github@gmail.com>
Both `hv-screen` and `hv-route` have the potential to have a null `doc` in state. This can happen from a bad load, or from an override document being provided by a parent component. The code used in `onUpdate` and its related methods were not accounting for the possibility of a null `doc'. - Updated `ScreenState` to reflect the values of the state as assigned in `hv-screen` - Updated callback signature to allow `getDoc` to return null - Updated `hv-root` implementation of `onUpdate` and related to account for null doc - Updated `HvGetRoot` to account for null doc - Updated behavior implementations which used `HvGetRoot` to account for null doc Asana: https://app.asana.com/0/0/1205741965789643/f
Implement `hv-route` as a provider of `onUpdate` to allow navigators to process behaviors Only routes which own a document are providers, any children which do not own a document will inherit. Asana: https://app.asana.com/0/1204008699308084/1205741965789637/f --------- Co-authored-by: Florent Bonomo <flochtililoch+github@gmail.com>
…#730) Implementation of `<behavior>` elements as children of `<navigator>` elements - Added an optional `<behavior>` child in schema - Gather and cache all "load" behaviors per navigator - warn developer of any behaviors with non matching triggers - trigger all behaviors on component load - Refresh and re-trigger behaviors on content update See individual commits for a few clean up steps performed as part of the work Asana: https://app.asana.com/0/1204008699308084/1205741965790735/f
Deep link requests were not properly triggering navigation events like tab selections. Behaviors continued to work as expected. Cause: There was a chunk of code which was calling `useContext` in the hv-navigator for two contexts. The result of these were not being used so [the code was removed](f3e25c0). Without that context, the `useEffect` of the custom navigators doesn't fire. Adding it back as part of the hierarchy restores the functionality. In the videos below, the deep link should cause a selection of the "Accounts" tab. | Before | After | | -- | -- | |  |  |
There was a problem hiding this comment.
Where are the event listeners added when loaded in the context of a navigator? From the change, I'm under the impression that we're only changing the "validation" aspect.
Also side note, if we go with this change, there's a few changes that will need to be done (presumably triggerLoadBehaviors will need to be either renamed or limited to filtering out load behaviors, and the comment above updateBehaviorElements is also specific to "load")
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Deep links would benefit from allowing
on-eventtriggered behaviors