ui-kit icon indicating copy to clipboard operation
ui-kit copied to clipboard

Formatting Time

Open montezume opened this issue 6 years ago • 0 comments

Relates to #959

While looking into #1019, I noticed weird behaviour with our TimeInputs. If you look at the following test


  it('should format the value when input is blurred on german locale', () => {
    const onChange = jest.fn();
    const { container } = render(
      <TimeInput {...baseProps} onChange={onChange} value="12:3" />,
      { locale: 'de' }
    );

    container.querySelector('input').focus();
    expect(container.querySelector('input')).toHaveFocus();
    container.querySelector('input').blur();
    expect(container.querySelector('input')).not.toHaveFocus();
    expect(onChange).toHaveBeenCalledWith({
      target: {
        id: expect.stringMatching(/^time-input-/i),
        name: undefined,
        value: '12:03',
      },
    });
  });

It looks good. Because it's a test. But look closer.

Why should I expect "12:3" to turn into "12:03"? It makes no sense. It should turn into "12:30". We have the same behaviour in english as well.

  it('should format the value when input is blurred on english locale', () => {
    const onChange = jest.fn();
    const { container } = render(
      <TimeInput {...baseProps} onChange={onChange} value="2:3 AM" />
    );

    container.querySelector('input').focus();
    expect(container.querySelector('input')).toHaveFocus();
    container.querySelector('input').blur();
    expect(container.querySelector('input')).not.toHaveFocus();
    expect(onChange).toHaveBeenCalledWith({
      target: {
        id: expect.stringMatching(/^time-input-/i),
        name: undefined,
        value: '2:03 AM',
      },
    });
  });

Again, why?

I'm really, really, starting to think that formatting on blur Time was just a bad idea, and it opens a can of worms that we never really considered.

montezume avatar Aug 26 '19 09:08 montezume