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

Dynamic nodes prevent navigation tree mutation #321

Merged
Merged
Show file tree
Hide file tree
Changes from 113 commits
Commits
Show all changes
117 commits
Select commit Hold shift + click to select a range
ec5f47c
tets
pekura Aug 6, 2018
1f7802a
Revert "tets"
pekura Aug 6, 2018
830407f
Merge remote-tracking branch 'upstream/master'
pekura Aug 8, 2018
7e110fa
Merge remote-tracking branch 'upstream/master'
pekura Aug 23, 2018
d600374
Merge remote-tracking branch 'upstream/master'
pekura Sep 11, 2018
e530f6d
Merge remote-tracking branch 'upstream/master'
pekura Oct 8, 2018
66776e8
Merge remote-tracking branch 'upstream/master'
pekura Oct 17, 2018
d4d3412
Merge remote-tracking branch 'upstream/master'
pekura Oct 17, 2018
fbc2301
Merge remote-tracking branch 'upstream/master'
pekura Oct 23, 2018
446bac3
Merge remote-tracking branch 'upstream/master'
pekura Oct 26, 2018
b589a3d
Merge remote-tracking branch 'upstream/master'
pekura Oct 31, 2018
ab1cc3f
Merge remote-tracking branch 'upstream/master'
pekura Nov 6, 2018
47d28b6
Merge remote-tracking branch 'upstream/master'
pekura Nov 12, 2018
aedd8da
Merge remote-tracking branch 'upstream/master'
pekura Nov 21, 2018
6656a2b
Modal interaction works
parostatkiem-zz Nov 22, 2018
3cf4917
tets
maxmarkus Oct 15, 2018
ccc8157
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Nov 23, 2018
a0c8f0e
Implement the prompt
parostatkiem-zz Nov 23, 2018
38afc97
Improve code & add comments
parostatkiem-zz Nov 23, 2018
6670654
Add support for custom title and text
parostatkiem-zz Nov 26, 2018
6b83b17
Fix obsolete import
parostatkiem-zz Nov 26, 2018
8abd146
Merge remote-tracking branch 'upstream/master'
maxmarkus Nov 26, 2018
2ea8f6b
Merge remote-tracking branch 'upstream/master'
pekura Nov 26, 2018
996844a
Luigi client creates extra entry in browser navigation history - fix.
pekura Nov 26, 2018
a6b091c
Merge remote-tracking branch 'upstream/master'
maxmarkus Nov 27, 2018
259a587
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Nov 27, 2018
19146ee
Handle potential errors
parostatkiem-zz Nov 27, 2018
eb1a472
Add unit test
parostatkiem-zz Nov 27, 2018
0f1f65e
Add demo to Angular example & add e2e tests
parostatkiem-zz Nov 27, 2018
6c91d1d
Update comment in e2e
parostatkiem-zz Nov 27, 2018
d7b3b8b
Undo temporary change
parostatkiem-zz Nov 27, 2018
a1f7843
Merge branch 'master' into Luigi-client-creates-extra-entry-in-browse…
maxmarkus Nov 27, 2018
12d000d
Merge branch 'master' into Luigi-client-creates-extra-entry-in-browse…
kwiatekus Nov 28, 2018
057c7ee
Merge remote-tracking branch 'upstream/master'
pekura Nov 28, 2018
1686dae
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Nov 28, 2018
ae77b09
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Nov 28, 2018
dd539e6
Merge remote-tracking branch 'upstream/master'
maxmarkus Nov 28, 2018
40c4a48
Merge remote-tracking branch 'upstream/master'
pekura Nov 28, 2018
bc3ffbe
Merge remote-tracking branch 'upstream/master'
pekura Nov 28, 2018
242c90f
Merge branch 'master' into Unsaved-changes-modal
jesusreal Nov 29, 2018
18e1ba6
Merge remote-tracking branch 'upstream/master'
maxmarkus Nov 29, 2018
87d0060
Remove unnecessary css imports
jesusreal Nov 30, 2018
84fc378
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Nov 30, 2018
22779f3
Add missing dependency
parostatkiem-zz Nov 30, 2018
d3293e3
Remove unnecessary changes
parostatkiem-zz Nov 30, 2018
4a51b86
Refactor state structure in app.html
jesusreal Nov 30, 2018
8165408
extract checking if component is able to handle modal
parostatkiem-zz Nov 30, 2018
488c767
Merge branch 'Unsaved-changes-modal' of https://github.com/parostatki…
parostatkiem-zz Nov 30, 2018
70fd297
shorten promise handling
parostatkiem-zz Nov 30, 2018
3c5820f
Remove obsolete changes
parostatkiem-zz Nov 30, 2018
9de58aa
Improve dirty page example in Angular app
parostatkiem-zz Nov 30, 2018
46ceedd
Improve solution
parostatkiem-zz Dec 3, 2018
4c7b559
Improve history handling
parostatkiem-zz Dec 3, 2018
ebf539c
Fix typo
parostatkiem-zz Dec 3, 2018
597f33f
Add missing dependency
parostatkiem-zz Dec 3, 2018
fa4943b
Improve e2e & use dedicated `data-cy` attribute
parostatkiem-zz Dec 3, 2018
13bcf62
Remove unnecessary e2e comment
parostatkiem-zz Dec 3, 2018
5852ca4
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Dec 3, 2018
d7b0cee
Merge remote-tracking branch 'upstream/master'
pekura Dec 4, 2018
4d92318
Merge branch 'master' into Luigi-client-creates-extra-entry-in-browse…
pekura Dec 4, 2018
7bf4fb3
hash navigation in FF fix
pekura Dec 4, 2018
9aa3241
context switcher on blur FF fix
pekura Dec 4, 2018
6a31016
do not show three dot menu on narrow screens if there is no displayab…
pekura Dec 4, 2018
2d3bd78
top nav links from the three dots menu did not work properly - fix
pekura Dec 4, 2018
92988f2
- fix content rendering in edge browser
pekura Dec 4, 2018
c33e2fc
Merge branch 'master' into Luigi-client-creates-extra-entry-in-browse…
pekura Dec 4, 2018
102e434
Merge remote-tracking branch 'upstream/master'
maxmarkus Dec 4, 2018
216c667
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Dec 5, 2018
0c014fe
Remove obsolete comment
parostatkiem-zz Dec 5, 2018
2bf41d9
rename 'changeDirtyStatus' method & generate proper docu
parostatkiem-zz Dec 5, 2018
1e51dfc
Rename e2e tests
parostatkiem-zz Dec 5, 2018
04f046a
Polish the language
parostatkiem-zz Dec 5, 2018
3e78486
Merge branch 'Luigi-client-creates-extra-entry-in-browser-navigation-…
maxmarkus Dec 5, 2018
9c5f6cb
Remove unnecessary comment
parostatkiem-zz Dec 5, 2018
ad5b55d
Center the modal when side nav is collapsed
parostatkiem-zz Dec 5, 2018
9e698c9
center the modal in the iframe
parostatkiem-zz Dec 5, 2018
eecd8a1
Improve modal responsiveness with side Nav
parostatkiem-zz Dec 5, 2018
a690e56
Remove unwanted history entry when user declines redirecting
parostatkiem-zz Dec 5, 2018
131bf8c
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Dec 5, 2018
7806a56
Avoid unwanted history entry when redirecting
parostatkiem-zz Dec 5, 2018
07fe6e6
Remove unnecessary example from docu
parostatkiem-zz Dec 5, 2018
ac311a9
Focus "Yes" button on modal
parostatkiem-zz Dec 5, 2018
f0ad156
Merge remote-tracking branch 'upstream/master'
maxmarkus Dec 5, 2018
77b76c7
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Dec 6, 2018
8997828
Improve modal positioning
parostatkiem-zz Dec 6, 2018
6f32df2
Merge remote-tracking branch 'upstream/master'
maxmarkus Dec 7, 2018
dbdd6d2
Merge branch 'master' of https://github.com/kyma-project/luigi into U…
parostatkiem-zz Dec 7, 2018
c502d65
typo in doc
maxmarkus Dec 7, 2018
47673ed
Improve responsiveness
parostatkiem-zz Dec 7, 2018
755576b
Merge remote-tracking branch 'upstream/master'
maxmarkus Dec 11, 2018
812a67d
Merge remote-tracking branch 'upstream/master'
maxmarkus Dec 11, 2018
8d1d9fa
Update unsaved changes modal trigger to support external links on top…
jesusreal Dec 12, 2018
3885d0b
Support unsaved changes modal trigger on logout
jesusreal Dec 12, 2018
ae830d0
Refactorings to minimize code duplication
jesusreal Dec 12, 2018
ff6f493
Add error handler for showUnsavedChangesModal on handleRouteClick mehtod
jesusreal Dec 12, 2018
71c8798
For navigation from luigi client, apply unsaved changes logic without…
jesusreal Dec 13, 2018
402f60e
Avoid needing to click twice in the modal when clicking history back …
jesusreal Dec 13, 2018
583356e
Fix unit tests
jesusreal Dec 13, 2018
d00da70
Merge remote-tracking branch 'upstream/master' into Unsaved-changes-m…
jesusreal Dec 13, 2018
1fc1b6d
Merge branch 'Unsaved-changes-modal' of github.com:parostatkiem/luigi
maxmarkus Dec 13, 2018
ba7d076
Merge remote-tracking branch 'upstream/master'
maxmarkus Jan 2, 2019
375678b
refactored findMatchingNode to not mutate the tree anymore
maxmarkus Jan 2, 2019
7256e38
path resolution working, match path fallback working, e2e tests green
maxmarkus Jan 7, 2019
239bbcb
made multiple path params work
maxmarkus Jan 8, 2019
a8ffb82
code cleanup
maxmarkus Jan 8, 2019
35e2658
fixed unit tests
maxmarkus Jan 8, 2019
0a9db70
added e2e test
maxmarkus Jan 8, 2019
cca92c6
Merge remote-tracking branch 'upstream/master' into dynamic-nodes-pre…
maxmarkus Jan 8, 2019
6ae9d06
moved some functions around, consolidated viewUrl substitution and ob…
maxmarkus Jan 9, 2019
3f33af2
refactored buildNode to pure function, fixed tets
maxmarkus Jan 9, 2019
41bb0fd
compare
maxmarkus Jan 9, 2019
0d085e7
Merge remote-tracking branch 'upstream/master' into dynamic-nodes-pre…
maxmarkus Jan 9, 2019
abeb231
removed spread operator for node names
maxmarkus Jan 9, 2019
9bfd741
using replaceVars instead of substituteViewUrl
maxmarkus Jan 11, 2019
3916f15
refactorings based on pr input
maxmarkus Jan 14, 2019
a3a8b27
Merge remote-tracking branch 'upstream/master' into dynamic-nodes-pre…
maxmarkus Jan 14, 2019
476b752
Merge remote-tracking branch 'upstream/master' into dynamic-nodes-pre…
maxmarkus Jan 14, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ describe('Luigi client features', () => {
{ path: '/projects/pr2/', successExpected: true },
// existent absolute path without '/' at the end
{ path: '/projects/pr2', successExpected: true },
// existent path with two dynamic pathSegments
{
path: '/projects/pr1/users/groups/avengers/settings/dynamic-two',
successExpected: true
},
// existent relative path
{ path: 'developers', successExpected: true }
].map(data => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ const routes: Routes = [
path: 'projects/:projectId/users/groups/:groupId/settings',
component: GroupSettingsComponent
},
{
path: 'projects/:projectId/users/groups/:groupId/settings/:dynamicValue',
component: DynamicComponent
},
{ path: 'projects/:projectId/developers', component: DevelopersComponent },
{ path: 'projects/:projectId/settings', component: SettingsComponent },
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,25 @@ var projectDetailNavProviderFn = function(context) {
{
label: 'Group Settings',
pathSegment: 'settings',
keepSelectedForChildren: true,
icon: 'user-settings',
viewUrl:
'/sampleapp.html#/projects/' +
projectId +
'/users/groups/:group/settings'
'/users/groups/:group/settings',
children: [
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look nice when clicked. I would recommend using the same approach as for the groups dynamic node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added keepSelectedForChildren: true, since we are not able to drill down here, because the child is dynamic. Think this is what you meant.

label: 'Multi Path Params',
pathSegment: ':dynamic',
viewUrl:
'/sampleapp.html#/projects/' +
projectId +
'/users/groups/:group/settings/:dynamic',
context: {
label: ':dynamic'
}
}
]
}
]
}
Expand Down
13 changes: 6 additions & 7 deletions core/src/App.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
import TopNav from './navigation/TopNav.html';
import LeftNav from './navigation/LeftNav.html';
import ConfirmationModal from './ConfirmationModal.html';
import { LuigiConfig } from './services/config.js';
import * as Routing from './services/routing.js';
import * as Navigation from './navigation/services/navigation.js'
import * as Iframe from './services/iframe';
import * as RoutingHelpers from './utilities/helpers/routing-helpers';
import * as GenericHelpers from './utilities/helpers/generic-helpers';
Expand Down Expand Up @@ -277,23 +279,20 @@
if ('luigi.navigation.pathExists' === e.data.msg) {
const data = e.data.data;
const path = buildPath(this, data);
const matchedPath = await Routing.matchPath(path);
const matched = await Routing.matchPath(path);

let normalizedPath =
(!path.startsWith('/') ? '/' : '') +
(path.endsWith('/') ? path.slice(0, -1) : path);
const pathExists = matchedPath === normalizedPath;
config.iframe.contentWindow.postMessage(
{
msg: 'luigi.navigation.pathExists.answer',
data: {
correlationId: data.id,
pathExists
pathExists: matched.pathData.isExistingRoute
}
},
'*'
);
}

if ('luigi.set-page-dirty' === e.data.msg) {
this.set({
unsavedChanges: {
Expand All @@ -320,7 +319,7 @@
},
handleNavClick(node) {
this.getUnsavedChangesModalPromise().then(
() => { Routing.handleRouteClick(node) }
() => { Routing.handleRouteClick(node, this.get()) }
);
},
handleModalResult(result) {
Expand Down
79 changes: 29 additions & 50 deletions core/src/navigation/services/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import * as NavigationHelpers from '../../utilities/helpers/navigation-helpers';
import * as AsyncHelpers from '../../utilities/helpers/async-helpers';
import * as GenericHelpers from '../../utilities/helpers/generic-helpers';
import * as RoutingHelpers from '../../utilities/helpers/routing-helpers';

export const getNavigationPath = async (rootNavProviderPromise, activePath) => {
if (!rootNavProviderPromise) {
Expand All @@ -25,12 +26,17 @@ export const getNavigationPath = async (rootNavProviderPromise, activePath) => {
}
await getChildren(rootNode); // keep it, mutates and filters children
const nodeNamesInCurrentPath = (activePath || '').split('/');
return buildNode(
const navObj = await buildNode(
nodeNamesInCurrentPath,
[rootNode],
rootNode.children,
rootNode.context || {}
);
navObj.isExistingRoute =
!activePath ||
nodeNamesInCurrentPath.length ===
navObj.navigationPath.filter(x => x.pathSegment).length;
return navObj;
} catch (err) {
console.error('Failed to load top navigation nodes.', err);
}
Expand Down Expand Up @@ -83,21 +89,23 @@ const buildNode = async (
nodeNamesInCurrentPath,
nodesInCurrentPath,
childrenOfCurrentNode,
context
context,
pathParams = {}
) => {
if (!context.parentNavigationContexts) {
context.parentNavigationContexts = [];
}
let result = {
navigationPath: nodesInCurrentPath,
context: context
context: context,
pathParams: pathParams
};
if (
nodeNamesInCurrentPath.length > 0 &&
childrenOfCurrentNode &&
childrenOfCurrentNode.length > 0
) {
const urlPathElement = nodeNamesInCurrentPath.splice(0, 1)[0];
const urlPathElement = nodeNamesInCurrentPath[0];
const node = findMatchingNode(urlPathElement, childrenOfCurrentNode);
if (node) {
nodesInCurrentPath.push(node);
Expand All @@ -109,11 +117,21 @@ const buildNode = async (
try {
let children = await getChildren(node, newContext);

if (node.pathSegment.startsWith(':')) {
pathParams[
node.pathSegment.replace(':', '')
] = RoutingHelpers.sanitizeParam(urlPathElement);
}
const newNodeNamesInCurrentPath = nodeNamesInCurrentPath.slice();
Copy link
Contributor

Choose a reason for hiding this comment

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

You can write it in just one line const newNodeNamesInCurrentPath = nodeNamesInCurrentPath.slice().shift(); and avoid the mutation of a variable defined as a constant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, since shift returns the removed item and I need to return everything except the removed item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But a .slice(1) is what I was searching for, thanks for pointing again to that issue.

newNodeNamesInCurrentPath.shift();

result = buildNode(
nodeNamesInCurrentPath,
newNodeNamesInCurrentPath,
// nodeNamesInCurrentPath,
nodesInCurrentPath,
children,
newContext
newContext,
pathParams
);
} catch (err) {
console.error('Error getting nodes children', err);
Expand Down Expand Up @@ -147,52 +165,13 @@ export const findMatchingNode = (urlPathElement, nodes) => {
}
}
nodes.some(node => {
// Static nodes
if (node.pathSegment === urlPathElement) {
result = node;
return true;
}

// Dynamic nodes
if (
(node.pathSegment && node.pathSegment.startsWith(':')) ||
(node.pathParam && node.pathParam.key)
// Static nodes
node.pathSegment === urlPathElement ||
// Dynamic nodes
(node.pathSegment && node.pathSegment.startsWith(':'))
) {
if (node.pathParam && node.pathParam.key) {
node.viewUrl = node.pathParam.viewUrl;
node.context = node.pathParam.context
? Object.assign({}, node.pathParam.context)
: undefined;
node.pathSegment = node.pathParam.pathSegment;
} else {
node.pathParam = {
key: node.pathSegment.slice(0),
pathSegment: node.pathSegment,
viewUrl: node.viewUrl,
context: node.context ? Object.assign({}, node.context) : undefined
};
}
node.pathParam.value = urlPathElement;

// path substitutions
node.pathSegment = node.pathSegment.replace(
node.pathParam.key,
urlPathElement
);

if (node.viewUrl) {
node.viewUrl = node.viewUrl.replace(node.pathParam.key, urlPathElement);
}

if (node.context) {
Object.entries(node.context).map(entry => {
const dynKey = entry[1];
if (dynKey === node.pathParam.key) {
node.context[entry[0]] = dynKey.replace(dynKey, urlPathElement);
}
});
}

// Return last matching node
result = node;
return true;
}
Expand Down
20 changes: 2 additions & 18 deletions core/src/services/iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
// Please consider adding any new methods to 'iframe-helpers' if they don't require anything from this file.
import * as IframeHelpers from '../utilities/helpers/iframe-helpers';
import * as GenericHelpers from '../utilities/helpers/generic-helpers';
import * as RoutingHelpers from '../utilities/helpers/routing-helpers';

const iframeNavFallbackTimeout = 2000;
const contextVarPrefix = 'context.';
const nodeParamsVarPrefix = 'nodeParams.';
let timeoutHandle;

export const getActiveIframe = node => {
Expand Down Expand Up @@ -39,22 +38,7 @@ export const navigateIframe = (config, component, node) => {
const componentData = component.get();
let viewUrl = componentData.viewUrl;
if (viewUrl) {
viewUrl = IframeHelpers.replaceVars(
viewUrl,
componentData.pathParams,
':',
false
);
viewUrl = IframeHelpers.replaceVars(
viewUrl,
componentData.context,
contextVarPrefix
);
viewUrl = IframeHelpers.replaceVars(
viewUrl,
componentData.nodeParams,
nodeParamsVarPrefix
);
viewUrl = RoutingHelpers.substituteViewUrl(viewUrl, componentData);
}

if (
Expand Down
Loading