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

Clearable time selector #18590

Merged
merged 12 commits into from
Jul 25, 2024
Merged

Clearable time selector #18590

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
abaa405
initial attempt at clearable time selector
schelv Nov 6, 2023
e230629
only show clear button if there is a value
schelv Nov 9, 2023
de78b87
use null instead of undefined.
schelv Nov 16, 2023
3f1ec34
leave event value undefined
schelv Dec 2, 2023
a1a7bee
Merge branch 'dev' into clearable-time-selector
bdraco Apr 19, 2024
a18a7eb
Merge branch 'dev' into clearable-time-selector
schelv Jul 13, 2024
4a8b154
move clear button into input
schelv Jul 13, 2024
122f0bf
add line
schelv Jul 17, 2024
f781395
Update src/components/ha-time-input.ts
schelv Jul 19, 2024
2956c1f
Merge branch 'dev' into clearable-time-selector
schelv Jul 20, 2024
5227541
improve label appearance
schelv Jul 20, 2024
274037e
remove the label values
schelv Jul 20, 2024
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
81 changes: 58 additions & 23 deletions src/components/ha-base-time-input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import "@material/mwc-list/mwc-list-item";
import { css, html, LitElement, TemplateResult } from "lit";
import { css, html, LitElement, TemplateResult, nothing } from "lit";
import { customElement, property } from "lit/decorators";
import { mdiClose } from "@mdi/js";
import { ifDefined } from "lit/directives/if-defined";
import { fireEvent } from "../common/dom/fire_event";
import { stopPropagation } from "../common/dom/stop_propagation";
import "./ha-select";
import "./ha-icon-button";
import { HaTextField } from "./ha-textfield";
import "./ha-input-helper-text";

Expand Down Expand Up @@ -77,27 +79,27 @@ export class HaBaseTimeInput extends LitElement {
/**
* Label for the day input
*/
@property() dayLabel = "";
@property() dayLabel = "Day";
silamon marked this conversation as resolved.
Show resolved Hide resolved

/**
* Label for the hour input
*/
@property() hourLabel = "";
@property() hourLabel = "Hour";

/**
* Label for the min input
*/
@property() minLabel = "";
@property() minLabel = "Minute";

/**
* Label for the sec input
*/
@property() secLabel = "";
@property() secLabel = "Second";

/**
* Label for the milli sec input
*/
@property() millisecLabel = "";
@property() millisecLabel = "Milisecond";

/**
* show the sec field
Expand All @@ -124,6 +126,8 @@ export class HaBaseTimeInput extends LitElement {
*/
@property() amPm: "AM" | "PM" = "AM";

@property({ type: Boolean, reflect: true }) public clearable?: boolean;

protected render(): TemplateResult {
return html`
${this.label
Expand Down Expand Up @@ -234,28 +238,40 @@ export class HaBaseTimeInput extends LitElement {
>
</ha-textfield>`
: ""}
${this.format === 24
? ""
: html`<ha-select
.required=${this.required}
.value=${this.amPm}
.disabled=${this.disabled}
name="amPm"
naturalMenuWidth
fixedMenuPosition
@selected=${this._valueChanged}
@closed=${stopPropagation}
>
<mwc-list-item value="AM">AM</mwc-list-item>
<mwc-list-item value="PM">PM</mwc-list-item>
</ha-select>`}
${this.clearable && !this.required && !this.disabled
? html`<ha-icon-button
label="clear"
@click=${this._clearValue}
.path=${mdiClose}
></ha-icon-button>`
: nothing}
</div>

${this.format === 24
? ""
: html`<ha-select
.required=${this.required}
.value=${this.amPm}
.disabled=${this.disabled}
name="amPm"
naturalMenuWidth
fixedMenuPosition
@selected=${this._valueChanged}
@closed=${stopPropagation}
>
<mwc-list-item value="AM">AM</mwc-list-item>
<mwc-list-item value="PM">PM</mwc-list-item>
</ha-select>`}
${this.helper
? html`<ha-input-helper-text>${this.helper}</ha-input-helper-text>`
: ""}
`;
}

private _clearValue(): void {
fireEvent(this, "value-changed");
}

private _valueChanged(ev: InputEvent) {
const textField = ev.currentTarget as HaTextField;
this[textField.name] =
Expand Down Expand Up @@ -302,18 +318,22 @@ export class HaBaseTimeInput extends LitElement {
}

static styles = css`
:host([clearable]) {
position: relative;
}
:host {
display: block;
display: flex;
silamon marked this conversation as resolved.
Show resolved Hide resolved
}
.time-input-wrap {
display: flex;
border-radius: var(--mdc-shape-small, 4px) var(--mdc-shape-small, 4px) 0 0;
overflow: hidden;
position: relative;
direction: ltr;
padding-right: 3px;
}
ha-textfield {
width: 40px;
width: 55px;
text-align: center;
--mdc-shape-small: 0;
--text-field-appearance: none;
Expand All @@ -335,6 +355,21 @@ export class HaBaseTimeInput extends LitElement {
--mdc-shape-small: 0;
width: 85px;
}
:host([clearable]) .mdc-select__anchor {
padding-inline-end: var(--select-selected-text-padding-end, 12px);
}
ha-icon-button {
position: relative
--mdc-icon-button-size: 36px;
--mdc-icon-size: 20px;
color: var(--secondary-text-color);
direction: var(--direction);
display: flex;
align-items: center;
background-color:var(--mdc-text-field-fill-color, whitesmoke);
border-bottom-style: solid;
border-bottom-width: 1px;
}
label {
-moz-osx-font-smoothing: grayscale;
-webkit-font-smoothing: antialiased;
Expand Down
1 change: 1 addition & 0 deletions src/components/ha-selector/ha-selector-time.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class HaTimeSelector extends LitElement {
.locale=${this.hass.locale}
.disabled=${this.disabled}
.required=${this.required}
clearable
.helper=${this.helper}
.label=${this.label}
enable-second
Expand Down
14 changes: 10 additions & 4 deletions src/components/ha-time-input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export class HaTimeInput extends LitElement {
@property({ type: Boolean, attribute: "enable-second" })
public enableSecond = false;

@property({ type: Boolean, reflect: true }) public clearable?: boolean;

protected render() {
const useAMPM = useAmPm(this.locale);

Expand All @@ -48,22 +50,26 @@ export class HaTimeInput extends LitElement {
@value-changed=${this._timeChanged}
.enableSecond=${this.enableSecond}
.required=${this.required}
.clearable=${this.clearable && this.value !== undefined}
.helper=${this.helper}
></ha-base-time-input>
`;
}

private _timeChanged(ev: CustomEvent<{ value: TimeChangedEvent }>) {
private _timeChanged(ev: CustomEvent<{ value?: TimeChangedEvent }>) {
ev.stopPropagation();
const eventValue = ev.detail.value;

const useAMPM = useAmPm(this.locale);
let value;
schelv marked this conversation as resolved.
Show resolved Hide resolved

// An undefined eventValue means the time selector is being cleared,
// the `value` variable will (intentionally) be left undefined.
if (
!isNaN(eventValue.hours) ||
!isNaN(eventValue.minutes) ||
!isNaN(eventValue.seconds)
eventValue !== undefined &&
(!isNaN(eventValue.hours) ||
!isNaN(eventValue.minutes) ||
!isNaN(eventValue.seconds))
Comment on lines +70 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Number.isNaN instead of isNaN.

isNaN is unsafe as it attempts a type coercion. Use Number.isNaN instead for better type safety.

-      (!isNaN(eventValue.hours) ||
-        !isNaN(eventValue.minutes) ||
-        !isNaN(eventValue.seconds))
+      (!Number.isNaN(eventValue.hours) ||
+        !Number.isNaN(eventValue.minutes) ||
+        !Number.isNaN(eventValue.seconds))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(!isNaN(eventValue.hours) ||
!isNaN(eventValue.minutes) ||
!isNaN(eventValue.seconds))
(!Number.isNaN(eventValue.hours) ||
!Number.isNaN(eventValue.minutes) ||
!Number.isNaN(eventValue.seconds))
Tools
Biome

[error] 70-70: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

[error] 71-71: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

[error] 72-72: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

Comment on lines +66 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Number.isNaN instead of isNaN.

isNaN is unsafe as it attempts a type coercion. Use Number.isNaN instead for better type safety.

-      (!isNaN(eventValue.hours) ||
-        !isNaN(eventValue.minutes) ||
-        !isNaN(eventValue.seconds))
+      (!Number.isNaN(eventValue.hours) ||
+        !Number.isNaN(eventValue.minutes) ||
+        !Number.isNaN(eventValue.seconds))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// An undefined eventValue means the time selector is being cleared,
// the `value` variable will (intentionally) be left undefined.
if (
!isNaN(eventValue.hours) ||
!isNaN(eventValue.minutes) ||
!isNaN(eventValue.seconds)
eventValue !== undefined &&
(!isNaN(eventValue.hours) ||
!isNaN(eventValue.minutes) ||
!isNaN(eventValue.seconds))
// An undefined eventValue means the time selector is being cleared,
// the `value` variable will (intentionally) be left undefined.
if (
eventValue !== undefined &&
(!Number.isNaN(eventValue.hours) ||
!Number.isNaN(eventValue.minutes) ||
!Number.isNaN(eventValue.seconds))
Tools
Biome

[error] 70-70: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

[error] 71-71: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

[error] 72-72: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

) {
let hours = eventValue.hours || 0;
if (eventValue && useAMPM) {
Expand Down
Loading