Keep left-side navigation on a Node hierarchy level #118

maxmarkus · 2018-10-01T11:20:57Z

keepSelectedForChildren Node setting for fixating the navigation.
Solves #93

… viewUrl, added some documentation and example implementation, overview link

…de example

…n is the configuration key

# Conflicts: # client/luigi-client.js # core/examples/luigi-sample-angular/src/app/app.module.ts # core/examples/luigi-sample-angular/src/app/overview/overview.component.ts # core/examples/luigi-sample-angular/src/app/project/dynamic/dynamic.component.spec.ts # core/examples/luigi-sample-angular/src/app/project/project.component.ts # core/src/services/navigation.js # core/test/navigation.spec.js # docs/lifecycle.md # docs/navigation-configuration.md

dariadomagala-sap

Just small fixes. Please read the comments :)

core/examples/luigi-sample-angular/src/app/project/dynamic/dynamic.component.ts

dariadomagala-sap · 2018-10-08T06:30:52Z

core/examples/luigi-sample-angular/e2e/tests/navigation.spec.js

+        .contains('keepSelectedForChildren')
+        .click();
+
+      const $body2 = $element.contents().find('body');


Could you please rename it to something more descriptive? 'body2' doesn't say too much.

dariadomagala-sap · 2018-10-08T06:38:40Z

core/examples/luigi-sample-angular/e2e/tests/navigation.spec.js

@@ -34,5 +34,37 @@ describe('Navigation', () => {
    });
    cy.get('.fd-app__sidebar').should('contain', 'First Child');
    cy.get('.fd-app__sidebar').should('contain', 'Second Child');
+
+    // keep selected for children example


I'm not sure if that's the best place for those tets. Maybe creating new one with just 'luigi-core features' would be a good idea?
I guess splitting tests for smaller pieces as soon as possible will be beneficial in the future :)

Agree to split the test into pieces. Since it is navigation feature, I'll leave it in the file.

dariadomagala-sap · 2018-10-08T06:41:26Z

core/examples/luigi-sample-angular/e2e/tests/navigation.spec.js

+      .click();
+
+    // dig into the iframe
+    cy.wait(50);


In my case 50 was sometimes not enough. Please increase this value.

dariadomagala-sap · 2018-10-08T06:55:50Z

core/src/services/navigation.js

+  return groupBy(nodes, 'category');
+};
+
+export const getTruncatedVirtualChildren = (children) => {


We agreed not tu use name 'vitrual children'. Please rename it :)

Same for 'virtualChildFound' and 'pathDataTruncatedVirtualChildren'

…erstanding

…hildren-93 # Conflicts: # README.md

dariadomagala-sap · 2018-10-08T07:47:31Z

core/examples/luigi-sample-angular/e2e/tests/navigation.spec.js

+        // on route change we need to refresh the contents() reference
+        iframeBody = $element.contents().find('body');
+        // wrap this body with cy so as to do cy actions inside iframe elements
+        const cyElement2 = cy.wrap(iframeBody);


Could you please rename this variable name as well? :)

dariadomagala-sap

LGTM

klaudiagrz · 2018-10-08T14:34:44Z

README.md

Why have you introduced "_" ? According to our guidelines, we format paths with code, so it should be: core/examples/luigi-sample-angular as in the previous version. Or am I missing something?

Actually I am not sure, since this was in the master and I had code stuff, but saw underscores coming from master. Then I'll change it back to code, thanks

klaudiagrz · 2018-10-08T14:38:20Z

docs/navigation-configuration.md

@@ -73,6 +73,7 @@ window.Luigi.setConfig({
 - **context** sends the specified object as context to the view. Use this parameter in combination with the dynamic **pathSegment** to receive the context through the context listeners of **Luigi client**. This is an alternative to using the dynamic value in the **viewUrl**.
 - **defaultChildNode** sets the child node that Luigi activates automatically if the current node has no **viewUrl** defined. Provide **pathSegment** of the child node you want to activate as a string.
 - **isolateView** renders the view in a new frame when you enter and leave the Node. This setting overrides the same-domain frame re-usage. The **isolateView** is disabled by default.
+- **keepSelectedForChildren** fixates the navigation in its current hierarchy, omitting display of children


- **keepSelectedForChildren** focuses the navigation in its current hierarchy, omitting the display of children.
I wouldn't use "fixate" here tbh.

klaudiagrz · 2018-10-08T14:39:55Z

core/examples/luigi-sample-angular/src/app/overview/overview.component.ts

@@ -25,12 +25,20 @@ export class OverviewComponent {
    {
      link: '/projects/pr1/dps',
      text: 'defaultChildNode',
-      description: 'navigation Node configuration to set a specific node as initial target'
+      description:
+        'navigation Node configuration to set a specific node as initial target'


Node as an initial target

klaudiagrz · 2018-10-08T14:40:10Z

core/examples/luigi-sample-angular/src/app/overview/overview.component.ts

    },
    {
      link: '/projects/pr1/users/groups',
      text: 'dynamic Nodes',
-      description: 'navigation Node configuration to set a specific node as dynamic'
+      description:
+        'navigation Node configuration to set a specific node as dynamic'


klaudiagrz · 2018-10-08T14:40:40Z

core/examples/luigi-sample-angular/src/app/overview/overview.component.ts

+      link: '/projects/pr1/avengers',
+      text: 'keepSelectedForChildren',
+      description:
+        'navigation Node configuration to fixate a navigation menu and omit the children'


~~fixate~~ -> focus on

klaudiagrz · 2018-10-09T11:16:36Z

docs/navigation-configuration.md

@@ -73,6 +73,7 @@ window.Luigi.setConfig({
 - **context** sends the specified object as context to the view. Use this parameter in combination with the dynamic **pathSegment** to receive the context through the context listeners of **Luigi client**. This is an alternative to using the dynamic value in the **viewUrl**.
 - **defaultChildNode** sets the child node that Luigi activates automatically if the current node has no **viewUrl** defined. Provide **pathSegment** of the child node you want to activate as a string.
 - **isolateView** renders the view in a new frame when you enter and leave the Node. This setting overrides the same-domain frame re-usage. The **isolateView** is disabled by default.
+- **keepSelectedForChildren** focuses the navigation in its current hierarchy, omitting the display of children.


focuses the navigation on

…hierarchy level (SAP#118) * initial dynamic node implementation. substitution for pathSegment and viewUrl and example implementation, overview link * removed obsolete routes and component, added child node to dynamic node example * added context substitution and enhanced config and example * moved leftnav business logic to navigation.js, keepSelectedForChildren is the configuration key * added unit, e2e tests * documentation update

maxmarkus and others added 30 commits July 31, 2018 13:51

Merge branch 'master' of github.com:kyma-project/luigi

41bb5f8

Merge branch 'master' of github.com:kyma-project/luigi

4349a3e

Merge branch 'master' of github.com:kyma-project/luigi

7cafa4b

Merge branch 'master' of github.com:kyma-project/luigi

f8a6a28

Merge branch 'master' of github.com:kyma-project/luigi

0ab8b3a

Merge branch 'master' of github.com:kyma-project/luigi

9e4e1f0

Merge branch 'master' of github.com:kyma-project/luigi

0d886e5

Merge branch 'master' of github.com:kyma-project/luigi

c0edce7

Merge branch 'master' of github.com:kyma-project/luigi

21fa6a4

Merge branch 'master' of github.com:kyma-project/luigi

63c3a61

Merge branch 'master' of github.com:kyma-project/luigi

c0d06bc

Merge branch 'master' of github.com:kyma-project/luigi

282ddcc

Merge branch 'master' of github.com:kyma-project/luigi

7623ae3

Merge branch 'master' of github.com:kyma-project/luigi

db5803f

Merge remote-tracking branch 'upstream/master'

ab060d9

Merge branch 'master' of github.com:kyma-project/luigi

a8be44f

Merge remote-tracking branch 'upstream/master'

f7b37bc

initial dynamic node implementation. substitution for pathSegment and…

7f4e729

… viewUrl, added some documentation and example implementation, overview link

removed obsolete routes and component, added child node to dynamic no…

944e6be

…de example

added context substitution and enhanced config and example

4ce51e6

added test for findMatchingNode

0eb115e

small fix for undefined param

49c8ce7

removed # from tets, added navigation tets for findMatchingNode

fd58d81

added functionality and example for virtual children feature

437f049

moved leftnav business logic to navigation.js, keepSelectedForChildre…

48e8c36

…n is the configuration key

Merge remote-tracking branch 'upstream/master'

dbd6180

added tests and fixed selectedNode bug

6f3eb4b

Merge remote-tracking branch 'upstream/master'

5833ec5

added e2e tests

6eff1e9

maxmarkus requested review from jesusreal, kazydek, klaudiagrz, kwiatekus, pekura, tomekpapiernik and y-kkamil as code owners October 1, 2018 11:20

maxmarkus changed the title ~~Feature/virtual children 93~~ Virtual children Node setting for keeping left-side navigation on a Node hierarchy level Oct 1, 2018

maxmarkus changed the title ~~Virtual children Node setting for keeping left-side navigation on a Node hierarchy level~~ Virtual children Node setting to keep left-side navigation on a Node hierarchy level Oct 1, 2018

dariadomagala-sap self-assigned this Oct 5, 2018

dariadomagala-sap suggested changes Oct 8, 2018

View reviewed changes

manually applied prettier on all the project files

d5c7661

maxmarkus requested a review from bszwarc as a code owner October 8, 2018 07:17

maxmarkus added 2 commits October 8, 2018 09:44

splitted feature tets in navigation, renamed some vars for better und…

7d6f695

…erstanding

Merge remote-tracking branch 'upstream/master' into feature/virtual-c…

4fe3c79

…hildren-93 # Conflicts: # README.md

dariadomagala-sap reviewed Oct 8, 2018

View reviewed changes

var renaming

0160330

dariadomagala-sap approved these changes Oct 8, 2018

View reviewed changes

klaudiagrz suggested changes Oct 8, 2018

View reviewed changes

documentation update

706a69a

klaudiagrz reviewed Oct 9, 2018

View reviewed changes

small docu update

102d229

klaudiagrz approved these changes Oct 9, 2018

View reviewed changes

maxmarkus added 2 commits October 10, 2018 11:33

Merge branch 'master' into feature/virtual-children-93

97bb178

Merge branch 'master' into feature/virtual-children-93

345766c

maxmarkus merged commit 167fb80 into SAP:master Oct 10, 2018

maxmarkus changed the title ~~Virtual children Node setting to keep left-side navigation on a Node hierarchy level~~ Keep left-side navigation on a Node hierarchy level Oct 10, 2018

maxmarkus deleted the feature/virtual-children-93 branch November 27, 2018 15:09

misappi unassigned dariadomagala-sap May 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep left-side navigation on a Node hierarchy level #118

Keep left-side navigation on a Node hierarchy level #118

maxmarkus commented Oct 1, 2018 •

edited

Loading

dariadomagala-sap left a comment

dariadomagala-sap Oct 8, 2018

dariadomagala-sap Oct 8, 2018

maxmarkus Oct 8, 2018

dariadomagala-sap Oct 8, 2018

dariadomagala-sap Oct 8, 2018

dariadomagala-sap Oct 8, 2018

dariadomagala-sap Oct 8, 2018

dariadomagala-sap left a comment

klaudiagrz Oct 8, 2018

maxmarkus Oct 8, 2018

klaudiagrz Oct 8, 2018

klaudiagrz Oct 8, 2018

klaudiagrz Oct 8, 2018

klaudiagrz Oct 8, 2018

klaudiagrz Oct 9, 2018

Keep left-side navigation on a Node hierarchy level #118

Keep left-side navigation on a Node hierarchy level #118

Conversation

maxmarkus commented Oct 1, 2018 • edited Loading

dariadomagala-sap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariadomagala-sap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxmarkus commented Oct 1, 2018 •

edited

Loading