This repository has been archived by the owner on Jun 1, 2024. It is now read-only.
Feature/navigation controllers #251
Merged
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
Tiagoperes
requested review from
carolinegoncalveszup,
leonardosantiagozup,
rafamsilva,
raphaelspimenta and
tarcisiogc
as code owners
This was referenced Sep 16, 2020
carolinegoncalveszup
previously approved these changes
Sep 18, 2020
hectorcustodiozup
previously approved these changes
Sep 18, 2020
…e/navigation-controllers
Tiagoperes
dismissed stale reviews from hectorcustodiozup and carolinegoncalveszup
via
e9ee07b
|
➿ Code coverage
|
pedronaveszup
approved these changes
Sep 18, 2020
carolinegoncalveszup
approved these changes
Sep 18, 2020
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
What I did
Closes #248
Issue summary:
We need to comply with the definition of
NavigationController, which can be changed via the NavigationActions.The NavigationController is the set of options responsible for the user feedback regarding the requests to navigate between views. In Beagle Web, they are:
A NavigationController should be attached to a navigation stack and it can be changed via the actions:
beagle:pushStack,beagle:resetStackandbeagle:resetApplication. An important point is that the navigationController to use is the controller corresponding to the current stack. So, we can't change the navigation history before we make the request to the new view, the navigation history can only be changed after successfully rendering the new view.In order to implement this, I had to do the following:
BeagleService:navigationControllers.controllerId.BeagleView.fetch has been deprecated
While resolving this issue, I noticed a big problem with our current navigation. The navigation history was completely decoupled from what was actually rendered in the BeagleView.
This is a problem because we could fall in the following situations:
BeagleView is started at
/home. The view rendered is/homeand the route at the top of the navigator is/home. A call is made tobeagleView.fetch({ path: '/products' }). Now the rendered view is/productswhile the view at the top of the navigator is/home, i.e., these two sources of truth contradict themselves.BeagleView is started at
/home. The view rendered is/homeand the route at the top of the navigator is/home. A call is made tobeagleView.getBeagleNavigator().pushView({ url: '/products' }). The rendered view is still/home, but now, the view at the top of the navigator is/products. Again, two sources of truth contradicting themselves.After much discussion between the team, we decided we have no need for
beagleView.fetchanymore. To fully change a view, we can now use the navigator, which, with this PR, also fetches the view and renders its content. To partially change a view we can use theLazyComponentand the actionbeagle:addChildren.The BeagleRemoteView (React and Angular)
With
beagleView.fetchnow deprecated and stuff likeshowLoadingandshowErrorbeing controlled by a navigation controller. It makes no sense to use the LoadParams as the parameter to create a BeagleRemoteView.Also there is a huge problem regarding the configuration of the requests via the
LoadParams. Every option set in the LoadParams are used only for the first request, every request after that completely ignores the parameters provided.We can also not change the behavior for people using the
LoadParams. So...LoadParamsas a parameter to the componentBeagleRemoteViewhas been deprecated. When used, it will work just like it was working previously, but it will warn the developer to consider changing to the new parameters.The new parameters for a BeagleRemoteView are:
route: replacespath. Tells the route to navigate to when the BeagleView is created. Whenrouteis first set, it generates apushView({ url: route })on the navigator. Whenrouteis changed, it generates aresetStack({ url: route })on the navigator.routeis not required, if it's not specified, no navigation will be done by default. This is interesting because today, we can manually control the BeagleView and in this case, it makes no sense to navigate by default.controllerId: tells which navigation controller should be used for first navigation stack. When not provided, the default navigation controller is used.networkOptions: new way of specifying options to all requests of the BeagleView. Differently fromLoadParams, this will be valid to every request and not just the first one. Acceptsheaders,strategyandmethod. When not specified, use the default options.The parameters
path, for React, and the parameterloadParams, for Angular, are no longer required. Deprecated parameters can't be required.Other problems fixed
methodof the functionTree.insertIntoTreewas not working whenmethod = 'replace'route.url, which is dangerous, some navigations might not have it.Tests
Related Pull Requests:
How I did it
beagleView.fetchand created new ways of specifying the request options.How to verify it
To verify it's all working, besides running the tests, I ran the POC beagle-forms for both Angular and React and ran the Playground.
Description for the changelog