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

feat(vwc-surface): viv-510 iss-859 complex menu surface #863

Merged
merged 35 commits into from
Jun 3, 2021

Conversation

gullerya
Copy link
Contributor

closes #859

This PR prides an vwc-surface component, which while being very similar to the vwc-menu is especially targeted to present complex contents in a surfaced fashion.

@gullerya gullerya added enhancement New feature or request Work in Progress You're welcome to have a look around and drop your early comments labels May 26, 2021
@github-actions
Copy link

🚀

Latest successful build of the PR deployed here.

🚀

@gullerya gullerya changed the title feat(vwc-surface): VIV 510 ISS 859 complex menu surface feat(vwc-surface): viv-510 iss-859 complex menu surface May 26, 2021
components/menu/src/vwc-menu.ts Outdated Show resolved Hide resolved
@gullerya gullerya requested a review from yinonov May 31, 2021 16:05
Copy link
Contributor

@yinonov yinonov left a comment

Choose a reason for hiding this comment

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

Can u trigger opening the menu in the stories on load?

components/dropdown/readme.md Outdated Show resolved Hide resolved
components/dropdown/src/vwc-dropdown.ts Outdated Show resolved Hide resolved
components/dropdown/src/vwc-dropdown.ts Outdated Show resolved Hide resolved
display: flex;
justify-content: flex-end;

::slotted(vwc-button) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
::slotted(vwc-button) {
::slotted(.vwc-button + .vwc-button) {

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've tried several syntaxes, nothing worked to me...

@@ -0,0 +1,23 @@
.dropdown-content {
padding: 24px;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest defining a variable as we discussed for each of the sections

:host {
  --vvd-dropdown-padding-inline: 24px;
  --vvd-dropdown-body-padding-inline: var(--vvd-dropdown-padding-inline); // or 0 as default?
  --vvd-dropdown-actions-padding-inline: var(--vvd-dropdown-padding-inline);
}

that way we have full control. what do you say?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented!

@gullerya gullerya added ready-to-merge and removed Work in Progress You're welcome to have a look around and drop your early comments labels Jun 2, 2021
@@ -14,13 +16,21 @@ declare global {

/* eslint-disable @typescript-eslint/ban-ts-comment */
// @ts-ignore
MWCMenu.styles = [styleCoupling, mwcMenuStyle, vwcMenuStyle];
MWCMenuBase.styles = [mwcMenuStyle, vwcMenuStyle];
Copy link
Contributor

Choose a reason for hiding this comment

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

styles were never applied to base classes. I believe we can finally set these in our own class peacefully since we extend the base class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point... I'll see if it breaks anything and will move to our own.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looked okay to me in StoryBook, implemented.

@sonarcloud
Copy link

sonarcloud bot commented Jun 3, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@gullerya gullerya merged commit 1520570 into master Jun 3, 2021
@gullerya gullerya deleted the viv-510/iss-859-complex-menu-surface branch June 3, 2021 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VIV-510 complex menu surface component
2 participants