Skip to content

Commit

Permalink
fix(tile): add hoverable attribute to avoid double focus in radio whe…
Browse files Browse the repository at this point in the history
…n interactive (#80)
  • Loading branch information
skhamvon authored Jun 26, 2023
1 parent d4b221f commit 9429178
Show file tree
Hide file tree
Showing 18 changed files with 79 additions and 138 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ export interface OdsTileAttributes extends OdsRadioizable, OdsCheckboxable, OdsC
disabled: boolean;
flex?: boolean;
/**
* If the tile can be have interactive pseudo-classes or not
* If the tile can have hoverable pseudo-classes or not
*/
interactive?: boolean;
hoverable?: boolean;
/**
* indicate if the tile has to be displayed in focused state
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,26 +80,6 @@ describe('spec:ods-tile-controller', () => {
});
});

describe('methods:getTabIndex', () => {
[false, true].forEach(disabled => {
[false, true].forEach(interactive => {
if (!disabled && interactive) {
it('should return 0 [false, true]', () => {
setup({disabled, interactive: interactive});
const index = controller.getTabIndex();
expect(index).toBe(0);
});
} else {
it(`should return -1 [${disabled}, ${interactive}]`, () => {
setup({disabled, interactive: interactive});
const index = controller.getTabIndex();
expect(index).toBe(-1);
});
}
});
});
});

describe('methods:handleClick', () => {
it('should call console.log', () => {
setup();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,6 @@ export class OdsTileController extends OdsComponentController<OdsTile> {
});
}

getTabIndex(): number {
if (!this.component.disabled && this.component.interactive) {
return 0;
}
return -1;
}

handleClick(): void {
// todo make a animation onclick
this.logger.log('[tile]', 'clicked');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const odsTileDefaultAttributesDoc = {
color: OdsThemeColorIntent.default,
disabled: false,
flex: false,
interactive: false,
hoverable: false,
hasFocus: false,
loading: false,
rounded: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class OdsTileMock implements OdsTile<OdsTileMethods, OdsTileEvents> {
color?: OdsThemeColorIntent;
disabled = false;
flex?: boolean;
interactive?: boolean;
hoverable?: boolean;
hasFocus = false;
loading = false;
rounded?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const odsTileBaseAttributes: OdsTileAttributes = {
checking: false,
color: OdsThemeColorIntent.default,
disabled: false,
interactive: false,
hoverable: false,
loading: false,
rounded: true,
size: OdsTileSize.md,
Expand Down
4 changes: 2 additions & 2 deletions packages/starters/react-vite/src/demo/Demo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ const Demo: React.FC = () => (
<h5>Ods Tile in radio group</h5>
<OsdsRadioGroup>
<OsdsRadio value="A">
<OsdsTile interactive>Radio-group with Radio & Tile in it (option A)</OsdsTile>
<OsdsTile hoverable>Radio-group with Radio & Tile in it (option A)</OsdsTile>
</OsdsRadio>
<OsdsRadio value="B">
<OsdsTile interactive>Radio-group with Radio & Tile in it (option B)</OsdsTile>
<OsdsTile hoverable>Radio-group with Radio & Tile in it (option B)</OsdsTile>
</OsdsRadio>
</OsdsRadioGroup>

Expand Down
4 changes: 2 additions & 2 deletions packages/starters/react/src/demo/Demo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ const Demo: React.FC = () => (
<h5>Ods Tile in radio group</h5>
<OsdsRadioGroup>
<OsdsRadio value="A">
<OsdsTile interactive>Radio-group with Radio & Tile in it (option A)</OsdsTile>
<OsdsTile hoverable>Radio-group with Radio & Tile in it (option A)</OsdsTile>
</OsdsRadio>
<OsdsRadio value="B">
<OsdsTile interactive>Radio-group with Radio & Tile in it (option B)</OsdsTile>
<OsdsTile hoverable>Radio-group with Radio & Tile in it (option B)</OsdsTile>
</OsdsRadio>
</OsdsRadioGroup>

Expand Down
4 changes: 2 additions & 2 deletions packages/starters/vue-vite/src/demo/Demo.vue
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ function handleMyButtonClick() {
<h5>Ods Tile in radio group</h5>
<OsdsRadioGroup>
<OsdsRadio value="A">
<OsdsTile interactive>Radio-group with Radio & Tile in it (option A)</OsdsTile>
<OsdsTile hoverable>Radio-group with Radio & Tile in it (option A)</OsdsTile>
</OsdsRadio>
<OsdsRadio value="B">
<OsdsTile interactive>Radio-group with Radio & Tile in it (option B)</OsdsTile>
<OsdsTile hoverable>Radio-group with Radio & Tile in it (option B)</OsdsTile>
</OsdsRadio>
</OsdsRadioGroup>

Expand Down
12 changes: 12 additions & 0 deletions packages/stencil/components/radio/src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@
</osds-radio-group>
<p id="radio-group-a-info"></p>


<p>radio with interactive or hoverable component inside</p>
<osds-radio-group id="radio-group-b">
<osds-radio value="val-tile1">
<osds-tile hoverable="true">myTile1</osds-tile>
</osds-radio>
<osds-radio value="val-tile2">
<osds-tile interactive="true">myTile2</osds-tile>
</osds-radio>
</osds-radio-group>


<script type="module">
document.addEventListener('odsInitialized', ({detail}) => {
const instance = detail.instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ describe('e2e:osds-tile', () => {
actionDescription: "disabled",
action: () => el.setProperty("disabled", true)
}, {
actionDescription: "interactive",
action: () => el.setProperty("interactive", true)
actionDescription: "hoverable",
action: () => el.setProperty("hoverable", true)
},
// TODO: don't test screenshot animated screen without freeze it
// {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
}
}

[interactive]:not([disabled]) {
[hoverable]:not([disabled]) {
@include osds-tile-on-selected-host() {
cursor: pointer;
}
Expand Down Expand Up @@ -109,19 +109,12 @@ slot[name="end"] {
background-position: 0 0;
}
}

}

}

// apply the theme template for the component
@include ods-theme-component() {
@include osds-tile-theme-color();
@include osds-tile-theme-size();
@include osds-tile-theme-typography();

:host([interactive]:focus-visible) {
outline-style: dashed;
outline-width: 2px;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,12 @@ describe('spec:osds-tile', () => {
});
});

describe('interactive', () => {
odsUnitTestAttribute<OdsTileAttributes, 'interactive'>({
...getAttributeContextOptions<OdsTileAttributes, OsdsTile, 'interactive'>({
name: 'interactive',
describe('hoverable', () => {
odsUnitTestAttribute<OdsTileAttributes, 'hoverable'>({
...getAttributeContextOptions<OdsTileAttributes, OsdsTile, 'hoverable'>({
name: 'hoverable',
list: [false, true],
defaultValue: odsTileDefaultAttributes.interactive,
defaultValue: odsTileDefaultAttributes.hoverable,
...config
})
});
Expand All @@ -186,12 +186,6 @@ describe('spec:osds-tile', () => {
expect(controller.validateAttributes).toHaveBeenCalledTimes(1);
});

it('should call controller.getTabIndex', async () => {
await setup();
expect(controller.getTabIndex).toHaveBeenCalledWith();
expect(controller.getTabIndex).toHaveBeenCalledTimes(1);
});

it('should call controller.handleClick on component click', async () => {
await setup();
root?.click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ export class OsdsTile implements OdsTile<OdsStencilMethods<OdsTileMethods>, OdsS
/** @see OdsTileAttributes.rounded */
@Prop({ reflect: true }) public rounded?: boolean = odsTileDefaultAttributes.rounded;

/** @see OdsTileAttributes.interactive */
@Prop({ reflect: true }) public interactive?: boolean = odsTileDefaultAttributes.interactive;
/** @see OdsTileAttributes.hoverable */
@Prop({ reflect: true }) public hoverable?: boolean = odsTileDefaultAttributes.hoverable;

/** @see OdsTileAttributes.loading */
@Prop({ reflect: true }) public loading: boolean = odsTileDefaultAttributes.loading;
Expand Down Expand Up @@ -73,7 +73,6 @@ export class OsdsTile implements OdsTile<OdsStencilMethods<OdsTileMethods>, OdsS
return (
<Host {...{
onClick: () => this.controller.handleClick(),
tabindex: this.controller.getTabIndex(),
}}>
<slot name={'start'}></slot>
<span class={'tile__centered-text'}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
}
}

&[interactive]:not([disabled]) {
&[hoverable]:not([disabled]) {
@include ods-and-one-hue-foreach-theme-color('100') using($color) {
:host(&:focus), :host(&:hover), :host(&[has-focus]) {
:host(&:hover) {
border-color: $color;
background-color: $color;
}
Expand Down Expand Up @@ -54,9 +54,9 @@
}
}

&[interactive]:not([disabled]) {
&[hoverable]:not([disabled]) {
@include ods-and-one-hue-foreach-theme-color('100') using($color) {
:host(&:focus), :host(&:hover), :host(&[has-focus]) {
:host(&:hover) {
background-color: $color;
}
}
Expand All @@ -69,7 +69,7 @@

&[checked] {
@include ods-and-one-hue-foreach-theme-color('500') using($color) {
:host(&), :host(&:focus), :host(&:hover), :host(&[has-focus]) {
:host(&), :host(&:hover) {
border-color: $color;
}
}
Expand Down Expand Up @@ -99,9 +99,9 @@
}
}

&[interactive]:not([disabled]) {
&[hoverable]:not([disabled]) {
@include ods-and-one-hue-foreach-theme-color('100') using($color) {
:host(&:focus), :host(&:hover), :host(&[has-focus]) {
:host(&:hover) {
border-color: $color;
background-color: $color;
}
Expand All @@ -116,7 +116,7 @@

&[checked] {
@include ods-and-one-hue-foreach-theme-color('500') using($color) {
:host(&), :host(&:focus), :host(&:hover), :host(&[has-focus]) {
:host(&), :host(&:hover) {
border-color: $color;
}
}
Expand Down Expand Up @@ -148,9 +148,9 @@
}
}

&[interactive]:not([disabled]) {
&[hoverable]:not([disabled]) {
@include ods-and-one-hue-foreach-theme-color('100') using($color) {
:host(&:focus), :host(&:hover), :host(&[has-focus]){
:host(&:hover){
background-color: transparent;
border-color: $color;
}
Expand All @@ -165,7 +165,7 @@

&[checked] {
@include ods-and-one-hue-foreach-theme-color('500') using($color) {
:host(&), :host(&:focus), :host(&:hover), :host(&[has-focus]) {
:host(&), :host(&:hover) {
border-color: $color;
}
}
Expand Down Expand Up @@ -226,24 +226,4 @@
[variant='hollow'] {
@include osds-tile-theme-color-variant-hollow();
}

:not([disabled]):focus,:not([disabled])[has-focus] {
@include osds-tile-on-selected-host() {
outline-color: var(--ods-color-gray-500);
}
}

:not([disabled])[contrasted]:focus,:not([disabled])[contrasted][has-focus] {
@include osds-tile-on-selected-host() {
outline-color: var(--ods-color-gray-000);
}
}

[interactive] {
@include ods-and-one-hue-foreach-theme-color('500') using($color) {
:host(&:focus-visible) {
outline-color: $color;
}
}
}
}

This file was deleted.

4 changes: 0 additions & 4 deletions packages/stencil/components/tile/src/docs/osds-tile/usage.mdx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import CheckedProperty from '@ovhcloud/ods-stencil-tile/src/docs/osds-tile/properties/usage.checked.mdx';
import DisabledProperty from '@ovhcloud/ods-stencil-tile/src/docs/osds-tile/properties/usage.disabled.mdx';
import InteractiveProperty from '@ovhcloud/ods-stencil-tile/src/docs/osds-tile/properties/usage.interactive.mdx';
import SizeProperty from '@ovhcloud/ods-stencil-tile/src/docs/osds-tile/properties/usage.size.mdx';
import VariantProperty from '@ovhcloud/ods-stencil-tile/src/docs/osds-tile/properties/usage.variant.mdx';
import RoundedProperty from '@ovhcloud/ods-stencil-tile/src/docs/osds-tile/properties/usage.rounded.mdx';
Expand Down Expand Up @@ -44,9 +43,6 @@ import GenericStyle from '@ovhcloud/ods-core/docs/generic-style.mdx';
### Size
<SizeProperty />

### Interactive
<InteractiveProperty />

### Checked
<CheckedProperty />

Expand Down
Loading

0 comments on commit 9429178

Please sign in to comment.