Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 7 additions & 0 deletions renderers/angular/src/v0_8/components/datetime-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ import { Types } from '../types';
:host {
display: block;
}
input {
color: #333;
}
input::-webkit-datetime-edit,
input::-webkit-datetime-edit-fields-wrapper {
color: #333;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To avoid repetition and improve maintainability, you can combine these CSS rules since they share the same declaration.

Suggested change
input {
color: #333;
}
input::-webkit-datetime-edit,
input::-webkit-datetime-edit-fields-wrapper {
color: #333;
}
input,
input::-webkit-datetime-edit,
input::-webkit-datetime-edit-fields-wrapper {
color: #333;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, combined selectors in 811f6da.

`,
changeDetection: ChangeDetectionStrategy.OnPush,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ import { BoundProperty } from '../../core/types';
border: 1px solid #ccc;
font-family: inherit;
flex: 1;
color: #333;
}
.a2ui-date-time-input::-webkit-datetime-edit,
.a2ui-date-time-input::-webkit-datetime-edit-fields-wrapper {
color: #333;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The color #333 is duplicated. To improve maintainability, consider defining a CSS custom property on a parent element (e.g., .a2ui-date-time-container) and reusing it.

For example:

.a2ui-date-time-container {
  --placeholder-color: #333;
  /* ... other styles */
}

.a2ui-date-time-input {
  /* ... other styles */
  color: var(--placeholder-color);
}

.a2ui-date-time-input::-webkit-datetime-edit,
.a2ui-date-time-input::-webkit-datetime-edit-fields-wrapper {
  color: var(--placeholder-color);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think a CSS custom property is overkill here since it's a single value in a small block. The combined selectors already reduce the duplication.

`,
],
Expand Down
6 changes: 6 additions & 0 deletions renderers/lit/src/0.8/ui/datetime-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ export class DateTimeInput extends Root {
padding: 8px;
border: 1px solid #ccc;
width: 100%;
color: #333;
}

input::-webkit-datetime-edit,
input::-webkit-datetime-edit-fields-wrapper {
color: #333;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The color #333 is duplicated. To improve maintainability, consider defining a CSS custom property on :host and reusing it.

For example:

:host {
  --placeholder-color: #333;
  /* ... other styles */
}

input {
  /* ... other styles */
  color: var(--placeholder-color);
}

input::-webkit-datetime-edit,
input::-webkit-datetime-edit-fields-wrapper {
  color: var(--placeholder-color);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same reasoning as above - combined selectors address the duplication without introducing a custom property for a single value.

`,
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { html, nothing } from "lit";
import { css, html, nothing } from "lit";
import { customElement } from "lit/decorators.js";
import { DateTimeInputApi } from "@a2ui/web_core/v0_9/basic_catalog";
import { A2uiLitElement, A2uiController } from "@a2ui/lit/v0_9";
Expand All @@ -27,6 +27,17 @@ export class A2uiDateTimeInputElement extends A2uiLitElement<
return new A2uiController(this, DateTimeInputApi);
}

static styles = css`
input {
color: #333;
}

input::-webkit-datetime-edit,
input::-webkit-datetime-edit-fields-wrapper {
color: #333;
}
`;
Comment on lines +30 to +36
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To avoid repetition and improve maintainability, you can combine these CSS rules since they share the same declaration.

  static styles = css`
    input,
    input::-webkit-datetime-edit,
    input::-webkit-datetime-edit-fields-wrapper {
      color: #333;
    }
  `;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, combined selectors in 811f6da.


render() {
const props = this.controller.props;
if (!props) return nothing;
Expand Down
5 changes: 5 additions & 0 deletions renderers/react/src/v0_8/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ export const componentSpecificStyles: string = `
padding: 8px;
border: 1px solid #ccc;
width: 100%;
color: #333;
}
:where(.a2ui-surface .a2ui-datetime-input) input::-webkit-datetime-edit,
:where(.a2ui-surface .a2ui-datetime-input) input::-webkit-datetime-edit-fields-wrapper {
color: #333;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The color #333 is duplicated. To improve maintainability, consider defining a CSS custom property on a parent selector (e.g., .a2ui-surface .a2ui-datetime-input) and reusing it.

For example:

.a2ui-surface .a2ui-datetime-input {
  --placeholder-color: #333;
  /* ... other styles */
}

:where(.a2ui-surface .a2ui-datetime-input) input {
  /* ... other styles */
  color: var(--placeholder-color);
}

:where(.a2ui-surface .a2ui-datetime-input) input::-webkit-datetime-edit,
:where(.a2ui-surface .a2ui-datetime-input) input::-webkit-datetime-edit-fields-wrapper {
  color: var(--placeholder-color);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as above - combined selectors reduce repetition, custom property not needed for a single value.


.a2ui-surface *,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ export const DateTimeInput = createReactComponent(DateTimeInputApi, ({props}) =>
border: STANDARD_BORDER,
borderRadius: STANDARD_RADIUS,
boxSizing: 'border-box',
color: '#333',
};

// CSS class scoped to this instance for pseudo-element rules that
// cannot be expressed as React inline styles (WebKit date/time inputs).
const scopeClass = `a2ui-dti-${uniqueId.replace(/:/g, '')}`;

return (
<div
style={{
Expand All @@ -49,13 +54,20 @@ export const DateTimeInput = createReactComponent(DateTimeInputApi, ({props}) =>
margin: LEAF_MARGIN,
}}
>
<style>{`
.${scopeClass}::-webkit-datetime-edit,
.${scopeClass}::-webkit-datetime-edit-fields-wrapper {
color: #333;
}
`}</style>
{props.label && (
<label htmlFor={uniqueId} style={{fontSize: '14px', fontWeight: 'bold'}}>
{props.label}
</label>
)}
<input
id={uniqueId}
className={scopeClass}
type={type}
style={style}
value={props.value || ''}
Expand Down