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

Support wc in split view #1664

Merged
merged 7 commits into from Nov 4, 2020
Merged

Support wc in split view #1664

merged 7 commits into from Nov 4, 2020

Conversation

ndricimrr
Copy link
Contributor

Description

Changes proposed in this pull request:

@ndricimrr ndricimrr added this to the Sprint 13 milestone Oct 15, 2020
@JohannesDoberer JohannesDoberer self-assigned this Oct 15, 2020
JohannesDoberer
JohannesDoberer reviewed Oct 19, 2020
View reviewed changes
Copy link
Contributor

@JohannesDoberer JohannesDoberer left a comment

Choose a reason for hiding this comment

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

Good Job 👍 .
Just one thing. Open a wc in splitview and after that (w/o closing the splitview) open a mf in splitview it is completely broken. Should we handle this (and protect the user from this missconfiguration) or consider it as missconfiguration?

@ndricimrr
Copy link
Contributor Author

ndricimrr commented Oct 20, 2020 •
edited

Good Job 👍 .
Just one thing. Open a wc in splitview and after that (w/o closing the splitview) open a mf in splitview it is completely broken. Should we handle this (and protect the user from this missconfiguration) or consider it as missconfiguration?

What do you get on your side: I get duplicates of the wc. Do you get the same?
I notice now that this is not an issue custom only to webcomponent in split view :
Trying this: Open iframe SplitView, open another one iframe in splitview , and you get two, in a similar behaviour.

I would guess if we decided to fix this then, should do it for master branch directly and not just this branch. can discuss in afterscrum

@JohannesDoberer JohannesDoberer added the enhancement New feature or request label Oct 20, 2020
JohannesDoberer
JohannesDoberer approved these changes Oct 20, 2020
View reviewed changes
Copy link
Contributor

@JohannesDoberer JohannesDoberer left a comment

Choose a reason for hiding this comment

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

👍

maxmarkus
maxmarkus suggested changes Oct 21, 2020
View reviewed changes
Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

Looks nice already, just some small things.

test/e2e-test-application/src/app/project/project.component.html Outdated Show resolved Hide resolved
core/src/SplitView.html Outdated Show resolved Hide resolved
@maxmarkus maxmarkus self-assigned this Oct 21, 2020
ndricimrr and others added 2 commits October 22, 2020 15:04
@ndricimrr @maxmarkus
Update test/e2e-test-application/src/app/project/project.component.html
5566984 
Co-authored-by: Markus <1720843+maxmarkus@users.noreply.github.com>
@ndricimrr
Update SplitView.html
f8d86bc
@JohannesDoberer JohannesDoberer merged commit 9c5cf20 into SAP:feature-wc-support Nov 4, 2020
@ndricimrr ndricimrr deleted the 1545-wc-splitview branch August 31, 2021 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxmarkus maxmarkus maxmarkus approved these changes

@JohannesDoberer JohannesDoberer JohannesDoberer approved these changes

@alexandra-simeonova alexandra-simeonova Awaiting requested review from alexandra-simeonova

@hardl hardl Awaiting requested review from hardl

@stanleychh stanleychh Awaiting requested review from stanleychh

@UlianaMunich UlianaMunich Awaiting requested review from UlianaMunich

Assignees

@maxmarkus maxmarkus

@JohannesDoberer JohannesDoberer

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@ndricimrr @maxmarkus @JohannesDoberer