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.

Sign up for GitHub

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

Jump to bottom

Drawer component #1659

Merged
merged 39 commits into from
Nov 16, 2020
Merged

Drawer component #1659

merged 39 commits into from
Nov 16, 2020

Conversation

JohannesDoberer
Copy link
Contributor

Resolves #1597

@JohannesDoberer
drawer
39566ea
@JohannesDoberer
Merge branch 'master' into drawer
52fc3b0 
* master:
  add link (SAP#1630)
  Add NextJS example link (SAP#1632)
  Feature accordion categories (SAP#1623)
  add disableInputHelpers for globalsearch (SAP#1620)
  Show more - blog posts (SAP#1625)
  fix link to the callback section (SAP#1628)
  Fix modal content not displaying issue in Safari (SAP#1624)
@JohannesDoberer
use modal as drawer
c9ed630
@JohannesDoberer
no message
2138b45
@JohannesDoberer
no message
390185c
@JohannesDoberer
no message
f38b0ad
@JohannesDoberer
no message
f388fab
@JohannesDoberer
Merge branch 'master' into poc-refactor-drawer
bebfb67 
* master:
  bump the versions of luigi packages (SAP#1649)
  Add NodeAccessibilityResolver tip to auth document (SAP#1639)
  Adding funtionality to logout the user dynamically without the need to click the logout button (SAP#1631)
  fix issues with brken left side nav in semiCollapsed mode and scroller (SAP#1633)
  Add ids and aria-labeldby to the nested lists (SAP#1607)
  fixed nav hrefs for hash routing (SAP#1640)
@JohannesDoberer
docu update
c14d2fa
@JohannesDoberer
remove code
139c20f
@JohannesDoberer JohannesDoberer added the enhancement New feature or request label Oct 12, 2020
@JohannesDoberer JohannesDoberer added this to the Sprint 13 milestone Oct 12, 2020
@ndricimrr ndricimrr self-assigned this Oct 14, 2020
@JohannesDoberer JohannesDoberer added the documentation documentation tasks label Oct 15, 2020
@alexandra-simeonova alexandra-simeonova self-assigned this Oct 15, 2020
@UlianaMunich UlianaMunich self-assigned this Oct 15, 2020
UlianaMunich
UlianaMunich reviewed Oct 16, 2020
View reviewed changes
Copy link
Contributor

@UlianaMunich UlianaMunich left a comment

Choose a reason for hiding this comment

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

Please check the issue with the broken view once I try to call the modal inside the drawer

core/src/Modal.html Outdated Show resolved Hide resolved
test/e2e-test-application/e2e/tests/1-angular/luigi-client-link-manager-features.spec.js Outdated Show resolved Hide resolved
UlianaMunich
UlianaMunich reviewed Nov 10, 2020
View reviewed changes
Copy link
Contributor

@UlianaMunich UlianaMunich left a comment

Choose a reason for hiding this comment

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

please check a couple of my comments. Other than that the functionality works fine for me

core/src/Drawer.html Outdated Show resolved Hide resolved
core/src/Drawer.html Outdated Show resolved Hide resolved
docs/luigi-client-api.md Outdated Show resolved Hide resolved
UlianaMunich
UlianaMunich reviewed Nov 10, 2020
View reviewed changes
core/src/Drawer.html Outdated Show resolved Hide resolved
UlianaMunich
UlianaMunich reviewed Nov 10, 2020
View reviewed changes
core/src/App.html Outdated Show resolved Hide resolved
UlianaMunich
UlianaMunich suggested changes Nov 10, 2020
View reviewed changes
Copy link
Contributor

@UlianaMunich UlianaMunich left a comment

Choose a reason for hiding this comment

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

2 minor suggestions

test/e2e-test-application/e2e/tests/1-angular/luigi-client-link-manager-features.spec.js Show resolved Hide resolved
@ndricimrr
Copy link
Contributor

ndricimrr commented Nov 12, 2020 •
edited
Loading

Edge 1:
I can see that it's producing a 'could not find the requested route undefined' for LuigiClient.linkManager().openAsDrawer('/'); , should we handle that ? 🤔
Screen Shot 2020-11-12 at 11 35 40

@ndricimrr
Copy link
Contributor

ndricimrr commented Nov 12, 2020 •
edited
Loading

Edge 2
Another question is do we want to deal with duplicates behavior, simply forbid it, or probably ignore...
Screen Shot 2020-11-12 at 11 44 40
Screen Shot 2020-11-12 at 11 43 08

@ndricimrr
Copy link
Contributor

ndricimrr commented Nov 12, 2020 •
edited
Loading

Edge 3:
If you try to 'Add Backdrop' from the main mf underneath the drawer, the prompt modal appears under the drawer and you would get stuck in that state with no way to get out. An edge case though, should we deal with it ?
Screen Shot 2020-11-12 at 11 52 48

@ndricimrr
Copy link
Contributor

ndricimrr commented Nov 12, 2020 •
edited
Loading

Edge 4:
Another edge case seems to be, trying to add a backdrop from inside a drawer, hides drawers header:
Screen Shot 2020-11-12 at 12 01 28
Screen Shot 2020-11-12 at 12 04 11

@ndricimrr
Copy link
Contributor

ndricimrr commented Nov 12, 2020 •
edited
Loading

Edge 5:
Another flow edge-case is.

  1. Open Drawer Example from LeftNav
  2. click Open Modal inside the drawer
  3. Trying to close the Modal from inside (scroll down to last button) also closes the Drawer at the end.

@ndricimrr
Copy link
Contributor

ndricimrr commented Nov 12, 2020 •
edited
Loading

Edge 6:
Another weird edge case, related to backdrop:
Steps below:
Screen Shot 2020-11-12 at 12 19 00
Screen Shot 2020-11-12 at 12 20 31
Steps 4 and 5 won't work, anything from inside the modal doesn't seem to work after this specific flow of events

@ndricimrr
Copy link
Contributor

Except for the first code suggestion, the rest are edge cases I just wanted to document here, so even if we don't address them at least we will be aware of that flow 👍

@JohannesDoberer
Copy link
Contributor Author

Edge 1: #1701
Edge 2: desired behaviour
Edge 3: Would be fixed with this issue #1702
Edge 4: desired behaviour
Edge 5: fixed in this pr
Edge 6: cannot reproduce, probably fixed with Edge5

@JohannesDoberer JohannesDoberer merged commit 186470d into SAP:master Nov 16, 2020
JohannesDoberer added a commit to hardl/luigi that referenced this pull request Nov 20, 2020
@JohannesDoberer
Merge branch 'master' into web-components-grid
e4c5693 
* master:
  Add openNodeInModal attribute to Profile section of the documentation (SAP#1704)
  Drawer component (SAP#1659)

# Conflicts:
#	core/src/App.html
#	core/src/Modal.html
#	test/e2e-test-application/src/app/project/project.component.html
#	test/e2e-test-application/src/luigi-config/extended/projectDetailNav.js
JohannesDoberer added a commit that referenced this pull request Nov 23, 2020
@JohannesDoberer
Merge branch 'master' into feature-wc-support
d63be09 
* master:
  Fix css issue for Iframe (#1716)
  Luigi Tutorials blog post (#1699)
  Add openNodeInModal attribute to Profile section of the documentation (#1704)
  Drawer component (#1659)
  Safari bug with shellbar dropdowns blue outlie (#1689)
  Bump versions yargs-parser and node-forge (#1683)

# Conflicts:
#	core/src/App.html
#	core/src/Modal.html
#	test/e2e-test-application/e2e/tests/1-angular/luigi-client-link-manager-features.spec.js
#	test/e2e-test-application/src/app/project/project.component.html
#	test/e2e-test-application/src/luigi-config/extended/projectDetailNav.js
@ndricimrr ndricimrr mentioned this pull request Nov 25, 2020
Merged
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
@JohannesDoberer
Drawer component (SAP#1659)
409e150
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndricimrr ndricimrr ndricimrr left review comments

@UlianaMunich UlianaMunich UlianaMunich approved these changes

@alexandra-simeonova alexandra-simeonova alexandra-simeonova approved these changes

@hardl hardl Awaiting requested review from hardl hardl is a code owner

@maxmarkus maxmarkus Awaiting requested review from maxmarkus

@stanleychh stanleychh Awaiting requested review from stanleychh

Assignees

@UlianaMunich UlianaMunich

@ndricimrr ndricimrr

@alexandra-simeonova alexandra-simeonova

Labels
documentation documentation tasks enhancement New feature or request
Projects
None yet
Milestone
Sprint 13
Development

Successfully merging this pull request may close these issues.

Implement a drawer component
4 participants
@JohannesDoberer @ndricimrr @UlianaMunich @alexandra-simeonova