Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSTR-228: Allow app custom routing #1005

Merged
merged 6 commits into from
Jun 21, 2022
Merged

MSTR-228: Allow app custom routing #1005

merged 6 commits into from
Jun 21, 2022

Conversation

guillegr123
Copy link
Contributor

This change allows to redirect to a registered app, if the first 2 parts of the hash route match a valid app, on load/re-load. For example:

  • apps/callcenter/queues/abc123/members redirects to apps/callcenter
  • apps/cluster/tasks redirects to apps/cluster

This enables Monster UI to load, reload or redirect to apps that use their own sub-routing system.

Normal routes (app/{appName}) and invalid routes should work as usual.

@guillegr123 guillegr123 self-assigned this May 13, 2022
@joristirado
Copy link
Contributor

joristirado commented May 14, 2022

Are some changes in this PR missing maybe?

monster.routing#hasMatch does not take any arguments: it only checks the current hash against apps/{appName}:?query.

@guillegr123
Copy link
Contributor Author

Sorry @azefiel , you are absolutely right, I forgot to commit the change for monster.routing#hasMatch 😅

It should be up now. Thanks for the heads up.

Copy link
Contributor

@joristirado joristirado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is the best way to go about this, as we have a dedicated library to abstract routing concerns.

Adding a new route pattern seems like a straightforward solution in this case.

It looks like crossroads's route registration facility has a flexible enough pattern matching mechanism to meet our needs (so called "rest segments" could be what we need here).

Happy to jump in a huddle any time to discuss this further if needed.

@guillegr123 guillegr123 changed the title MSTR-228: Redirect to app if path start is correct on load MSTR-228: Allow additional fragments for app routes Jun 6, 2022
@guillegr123
Copy link
Contributor Author

guillegr123 commented Jun 6, 2022

Hi @azefiel, I tried using the rest fragments, but it seems that Crossroads.js does not support having both an optional rest fragment and an optional query capturing group. So I ended up using a regular expression instead, and exposing the parseQueryString() function at monster.util, to convert the extracted query string into an object as Crossroads.js used to do.

src/js/lib/monster.routing.js Outdated Show resolved Hide resolved
src/js/lib/monster.routing.js Outdated Show resolved Hide resolved
src/js/lib/monster.util.js Show resolved Hide resolved
Use RegExp instead of pattern for app route, because Crossroads.js
extracts the query string as part of the rest segment. This is because
it does not support having both segments as optional.
See: millermedeiros/crossroads.js#130
Copy link
Contributor

@joristirado joristirado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last nitpicking, just want to make sure this code is as understandable as possible for our future self 😅

src/js/lib/monster.routing.js Outdated Show resolved Hide resolved
src/js/lib/monster.routing.js Outdated Show resolved Hide resolved
@guillegr123 guillegr123 changed the title MSTR-228: Allow additional fragments for app routes MSTR-228: Allow app custom routing Jun 20, 2022
@joristirado joristirado merged commit 2f9d80b into master Jun 21, 2022
@joristirado joristirado deleted the MSTR-228 branch June 21, 2022 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants