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

fix(elevation): sharp or rounded shape #1185

Merged
merged 4 commits into from
Jan 11, 2022
Merged

fix(elevation): sharp or rounded shape #1185

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9846996
fix(elevation): sharp or rounded shape
rachelbt Jan 2, 2022
7bf893e
Merge branch 'master' of https://github.com/Vonage/vivid into issue1179
rachelbt Jan 9, 2022
227bcac
code review changes
rachelbt Jan 9, 2022
0699366
Merge branch 'master' into issue1179
rachelbt Jan 11, 2022
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
1 change: 1 addition & 0 deletions common/foundation/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export enum ConnotationDecorative {

/* eslint-disable no-shadow */
export enum Shape {
Sharp = 'sharp',
Rounded = 'rounded',
Pill = 'pill',
Circled = 'circled',
Expand Down
11 changes: 7 additions & 4 deletions components/elevation/src/vwc-elevation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
@use 'sass:list';

$dps: 0 2 4 8 12 16 24;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be coming from tokens automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure what you mean. This is for the sass loop :)

Copy link
Contributor

Choose a reason for hiding this comment

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

probably not related to this PR, but the values are hardcoded here instead of being imported from style tokens

Copy link
Contributor Author

@rachelbt rachelbt Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That what was there - I merely added the 0 ;)

$vvd-elevation-background-color: --vvd-elevation-background-color;
Copy link
Contributor

Choose a reason for hiding this comment

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

These constants prevent typos that may occur by authoring "magic strings".
Even if used in a single page, renaming a variable may leave a reference outdated. While constants will break the compilation early

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 removed that line anyway :)

Copy link
Contributor

Choose a reason for hiding this comment

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

right, don't. it helps ensure we use the right css variable. rule of thumb

$vvd-elevation-border-radius: --vvd-elevation-border-radius;

@function get-default-selector($list, $default) {
@if list.length($list) == 1 {
Expand All @@ -20,13 +18,18 @@ $vvd-elevation-border-radius: --vvd-elevation-border-radius;

@each $dp in $dps {
#{get-default-selector($dps, $dp)} {
background-color: var(#{$vvd-elevation-background-color}, var(--vvd-color-surface-#{$dp}dp));
background-color: var(--vvd-color-surface-#{$dp}dp);
filter: var(--vvd-shadow-surface-#{$dp}dp);
}
}

.vwc-elevation {
--border-radius-size: 6px;
width: fit-content;
border-radius: var(#{$vvd-elevation-border-radius}, 6px);
border-radius: var(--border-radius-size);
contain: content;

&-sharp {
--border-radius-size: 0;
}
}
9 changes: 8 additions & 1 deletion components/elevation/src/vwc-elevation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ import { customElement, html, LitElement } from 'lit-element';
import { style } from './vwc-elevation.css.js';
import { property } from 'lit-element/lib/decorators.js';
import { classMap } from 'lit-html/directives/class-map.js';
import type { Shape } from '@vonage/vvd-foundation/constants.js';

declare global {
interface HTMLElementTagNameMap {
'vwc-elevation': VWCElevation;
}
}

type ElevationShape = Extract<Shape, Shape.Rounded>;

@customElement('vwc-elevation')
export class VWCElevation extends LitElement {
/**
Expand All @@ -20,9 +23,13 @@ export class VWCElevation extends LitElement {
@property({ type: Number, reflect: false })
dp = 2;

@property( {type: String, reflect: true })
shape?: ElevationShape[number] = 'Rounded';

protected override render(): unknown {
const classList = {
[`vwc-elevation-dp-${this.dp}`]: true
[`vwc-elevation-dp-${this.dp}`]: true,
[`vwc-elevation-${this.shape}`]: !!this.shape,
};

return html`
Expand Down
11 changes: 4 additions & 7 deletions components/elevation/stories/elevation.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,12 @@ const Template = args => html`
export const Basic = Template.bind({});
Basic.args = { label: 'Basic' };

export const Sharp = Template.bind({});
Sharp.args = { label: 'Sharp Corners' , shape: 'sharp', dp: '0' };

export const AllElevations = AllTemplate.bind({});
AllTemplate.args = { label: 'All' };

export const BackgroundColor = Template.bind({});
BackgroundColor.args = { label: 'Background Color', style: '--vvd-elevation-background-color: lightblue', dp: 8 };

export const BorderRadius = Template.bind({});
BorderRadius.args = { label: 'Border Radius', style: '--vvd-elevation-border-radius: 16px', dp: 24 };

const HoverEffectExampleTemplate = args => html`
${styles()}
<vwc-elevation ...=${spread(args)}
Expand All @@ -64,7 +61,7 @@ const HoverEffectExampleTemplate = args => html`
`;

export const HoverEffectExample = HoverEffectExampleTemplate.bind({});
HoverEffectExample.args = { label: 'Border Radius', style: '--vvd-elevation-border-radius: 16px', dp: 24 };
HoverEffectExample.args = { label: 'Border Radius', dp: 24 };

function onMouseEnter(e) {
e.target.setAttribute('dp', '24');
Expand Down
26 changes: 8 additions & 18 deletions ui-tests/tests/vwc-elevation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ export async function createElementVariations(wrapper) {
text-align: center;
}
</style>

<vwc-elevation shape="sharp" dp="0">
<div class="card">
This is the content inside the elevation with DP 0 and sharp edges
</div>
</vwc-elevation>


<vwc-elevation dp="0">
<div class="card">
This is the content inside the elevation with DP 2
Expand Down Expand Up @@ -61,24 +69,6 @@ export async function createElementVariations(wrapper) {
This is the content inside the elevation with DP 24
</div>
</vwc-elevation>

<vwc-elevation style="--vvd-elevation-border-radius: 16px; --vvd-elevation-background-color:lightblue;">
<div class="card">
This is the content inside the elevation with background and radius
</div>
</vwc-elevation>

<vwc-elevation>
<div class="card" style="background-color: lightpink; height: auto;">
This is the content inside the elevation with radius and inside element with background color
</div>
</vwc-elevation>

<vwc-elevation style="--vvd-elevation-border-radius: 0;">
<div class="card" style="background-color: lightpink; height: auto;">
This is the content inside the elevation with no border-radius
</div>
</vwc-elevation>
`;
wrapper.appendChild(elementWrapper);
}
Expand Down