Skip to content

fix(react): Small adjustments to some components.#1624

Open
ditman wants to merge 1 commit into
a2ui-project:mainfrom
ditman:react-minor-component-fixes
Open

fix(react): Small adjustments to some components.#1624
ditman wants to merge 1 commit into
a2ui-project:mainfrom
ditman:react-minor-component-fixes

Conversation

@ditman

@ditman ditman commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Adds a few classnames to some of the basic catalog components so they can be located in tests in a similar way to those on Lit and Angular: Card, ChoicePicker, Modal and Tabs.

Adds handling for the DateTimeInput value so it conforms with HTML5 values; similar to what was needed in the Lit package.

Pre-launch Checklist

If you need help, consider asking for advice on the discussion board.

Adds a few classnames to some of the basic catalog components so they
can be located in tests in a similar way to those on Lit and Angular:

Card, ChoicePicker, Modal and Tabs.

Adds handling for the DateTimeInput value so it conforms with HTML5
values.
@ditman ditman requested a review from josemontespg June 12, 2026 02:08

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request adds missing CSS classes to the Card, ChoicePicker, Modal, and Tabs components to align with other implementations, and updates DateTimeInput to support rendering different input types correctly. A bug was identified in the new normalizeDateTimeValue helper function where values without a 'T' separator (such as date-only or time-only strings) are parsed incorrectly, leading to invalid date or time values.

Comment on lines +22 to +40
function normalizeDateTimeValue(value: string | null | undefined, type: string): string {
if (!value) return '';

const hasT = value.includes('T');
const split = value.split('T');

const datePart = (hasT ? split[0] : value)?.substring(0, 10) ?? '';
const timePart = (hasT ? split[1] : value)?.substring(0, 5) ?? '';

switch (type) {
case 'date':
return datePart;
case 'time':
return timePart;
case 'datetime-local':
return `${datePart}T${timePart}`;
}
return '';
}

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.

high

The current implementation of normalizeDateTimeValue has a bug when the input value does not contain a 'T' separator (e.g., when it is a date-only string like "2026-03-30" or a time-only string like "12:00").

Specifically, if hasT is false, both datePart and timePart fall back to using the entire value. This results in:

  • A date-only string being sliced for the time part (e.g., "2026-03-30".substring(0, 5) becomes "2026-"), which is an invalid time value.
  • A time-only string being sliced for the date part (e.g., "12:00".substring(0, 10) becomes "12:00"), which is an invalid date value.

To fix this, we should check if the value contains a colon (:) to correctly identify and separate the date and time parts when 'T' is not present.

function normalizeDateTimeValue(value: string | null | undefined, type: string): string {
  if (!value) return '';

  let datePart = '';
  let timePart = '';

  if (value.includes('T')) {
    const [date, time] = value.split('T');
    datePart = date ?? '';
    timePart = time ?? '';
  } else if (value.includes(':')) {
    timePart = value;
  } else {
    datePart = value;
  }

  datePart = datePart.substring(0, 10);
  timePart = timePart.substring(0, 5);

  switch (type) {
    case 'date':
      return datePart;
    case 'time':
      return timePart;
    case 'datetime-local':
      return datePart ? datePart + 'T' + (timePart || '00:00') : '';
  }
  return '';
}

@ditman ditman mentioned this pull request Jun 12, 2026
7 tasks
@github-project-automation github-project-automation Bot moved this to Todo in A2UI Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant