Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

flow service: collected issues #373

Closed
3 of 5 tasks
alex3683 opened this issue Oct 11, 2016 · 3 comments
Closed
3 of 5 tasks

flow service: collected issues #373

alex3683 opened this issue Oct 11, 2016 · 3 comments

Comments

@alex3683
Copy link
Member

alex3683 commented Oct 11, 2016

  • 2.x: Check the target of didNavigate events: Currently if a place instead of a target is used as the value of the target field in navigateRequest events, this is also used as the topic of the didNavigate event. Since this violates the constraints of the topic syntax and is no target at all, we'd rather leave the value for target empty. In case of a direct request (i.e. the URL changes) _self is used, which is also incorrect (flow (v2.x): differentiate target and place in navigation events  #381).
  • 2.x: Make target and place explicit in navigateRequest events: Currently only target is supported, which could be a place or a target. This should be more explicit, so that either target or place is allowed, but each is only interpreted as what it name says it is. If both are given, this incident should be logged, but the place should eventually be favored, since its more explicit (flow (v2.x): differentiate target and place in navigation events  #381).
  • 2.x: When validating the flow.json against the flow JSON schema, defaults should directly be applied. Currently defaults are set manually (2.x: move from jjv to ajv for JSON schema validation #380).
  • For 1.x: after a redirect we should call $location.replace() to replace the entry on the history stack instead of putting it on top. Otherwise redirects create two entries and thus break the back button. For 2.x we should check the according api in page.js (flow: ensure that redirects don't mess with browser history #382) (edit: the issue does not exist in page.js, so nothing to do for 2.x).
  • 1.x: If the application uses a non empty base for routing in html5 mode, that base is added as prefix to the routes but also to the place name within the didNavigate event. This leads to inconsistency between configured places and places within the event. The context (i.e. the base) should never be part of the places. Have a look at flow.spec.js for an example (remove HTML5 router base from navigation events #383).
@x1B
Copy link
Member

x1B commented Oct 20, 2016

  • 2.x: we probably also need to double-encode slashes in path segments, like with AngularJS in v1.x (cf Double-Encoding issue with place parameters #379). When navigating to a URL, the browser will decode single-encoded slashes before page.js starts the route matching process, causing those slashes to break the match (flow: (v2.x) double-encode slashes in path segments #386).
  • 2.x: when a flow definition contains an invalid empty place, an error is logged (good). Afterwards however, a route is set up and the log message for the route does not seem to match the actual place that is used (if there is no global error redirect, the log message also seems wrong). IMO, we should simply return after logging the error and not try to do anything "useful" afterwards. Then we should consider when/if it makes sense to use the error redirect (flow (v2.x): simplify handling empty places #384).

@x1B
Copy link
Member

x1B commented Oct 26, 2016

Updated description and comments: added references to individual tickets.

@x1B x1B added the duplicate label Jan 12, 2017
@x1B
Copy link
Member

x1B commented Jan 12, 2017

Closing as duplicate: remaining issues are tracked in their respective issues

@x1B x1B closed this as completed Jan 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants