Skip to content
This repository was archived by the owner on Sep 13, 2021. It is now read-only.

Conversation

@wKich
Copy link
Member

@wKich wKich commented Mar 30, 2021

PoC API of Date/Time pickers

const DatePicker = /* HTML of FieldForm */.extend('datepicker')
  .filters({ disabled, readonly, valid })
  .actions({
    setDate({ year?: number, month?: number, day?: number })
    setYear(year: number)
    setMonth(month: number)
    setDay(day: number)
    clear() // Only possible if datepicker is allow to do it, throw an error overwise
    setToday() // The same as 'clear'
    setValue(value: string) // The same. 
  })

const TimePicker = /* HTML of FieldForm */.extend('timepicker')
  .filters({ disabled, readonly, valid, format /* 12/24 hours */ })
  .actions({
    setTime({ hour?: number, minute?: number, second?: number }, format?: 'am' | 'pm')
    setHour(hour: number, format?: 'am' | 'pm')
    setMinute(minute: number)
    setSecond(second: number)
    clear() // Only possible if datepicker is allow to do it, throw an error overwise
    setNow() // The same as 'clear'
    setValue(value: string) // The same
  })

I think the all set methods have to throw an error if date is beyond min/max date
And if Datepicker is disabled or is readonly

In some cases, datepicker might be configured to allow select only the year or month or day or any combination of these
So in that case I think we have to throw an error if a user is trying to set something that's not allowed

Progress:

  • DatePicker interactor
    • Calendar Interactor
    • Calendar with date utils
    • Calendar tests
    • DatePicker variants
      • Modal
      • Inline
      • Static
      • Views combinations (month/month+year/year)
      • KeyboardDatePicker
    • DatePicker with date utils
    • DatePicker tests (done only for simple variant)
  • TimePicker Interactor
    • Clock Interactor
  • DateTimePicker Interactor

@wKich wKich requested review from cowboyd and elrickvm March 30, 2021 11:11
@cowboyd
Copy link
Member

cowboyd commented Mar 30, 2021

Heavy lift here 💪🏻 💪🏻

keep it up!

@wKich wKich marked this pull request as ready for review April 1, 2021 08:21
@wKich
Copy link
Member Author

wKich commented Apr 1, 2021

@cowboyd @elrickvm, the calendar ready to review. There are some issues and questions. I described them in the comments, please, take a look.

@wKich wKich force-pushed the date-time-pickers branch from bd7c69c to d6f2e85 Compare April 6, 2021 10:46
@wKich wKich force-pushed the date-time-pickers branch from d6f2e85 to 081926f Compare April 6, 2021 11:14
src/calendar.ts Outdated
Comment on lines 31 to 37
return new Promise<number | undefined>((resolve) =>
interactor.perform((element) => {
const yearString = getYear(element);
const year = yearString ? parseInt(yearString) : NaN;
resolve(Number.isNaN(year) ? undefined : year);
})
);
Copy link

Choose a reason for hiding this comment

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

perform should really allow you to return a value out of it, I added an issue for this on bigtest: thefrontside/bigtest#914

@cowboyd
Copy link
Member

cowboyd commented Apr 9, 2021

@wKich As I was talking this over with @jnicklas it occured to me that while we can make heroic hacks to make this work in a locale-indepent way, the fact of the matter is that locale (and specifically date, time and month formatting) are a key abstraction to the component.

If we think of it that way, then there really is no one true "DatePicker" component, but instead an abstraction λlocale → DatePicker(locale) that can generate hundreds of potential datepicker components.

Perhaps the simplest thing that could possibly work would be to introduce this same abstraction of the DatePicker component into the DatePicker interactor so that it can also generate hundreds of potential datepicker interactors. In other words, λlocale → Interactor(locale)

We could do this in practice by providing not a DatePicker interator, but a createDatePicker() function that accepted formatting functions and returned a DatePicker.

import { createDatePickerInteractor } from 'material-ui-interactors';

const DatePicker = createDatePicker({
  get(formattedDate: string): Date {
    //parse date
  },
  set(date: Date): void {
  }
});

The interface doesn't need to look exactly like this, but you get the idea.

We are going to have to fight the demon named "locale" head on at some point, and this will be an incremental step in that direction. What do you think?

@wKich
Copy link
Member Author

wKich commented Apr 9, 2021

@cowboyd, I think it would be enough to pass date-io utils into the createDatePicker function. We can get a format from the data attribute and we have a value. And then we just need to use parse(value: string, format: string): TDate | null; from utils to get a date. But the problem is that the value can't have full information about the exact date. For example, the first example from the official demo https://material-ui-pickers.dev/demo/datepicker, has April 9th as a value, but lacks a year, so potentially it could be any year.

Datepicker usage examples and API of @material-ui/pickers

@cowboyd
Copy link
Member

cowboyd commented Apr 9, 2021

For example, the first example from the official demo https://material-ui-pickers.dev/demo/datepicker, has April 9th as a value, but lacks a year, so potentially it could be any year.

Hmm, this is interesting if we look at it from a type perspective, there are two different data types here Day, and Date I think we need to somehow fix what is the data type of the thing you're working with.

Datepicker usage examples and API of @material-ui/pickers

@wKich
Copy link
Member Author

wKich commented Apr 12, 2021

React-admin uses the DatePicker with the native variation and they don't expose format attribute
image

@cowboyd
Copy link
Member

cowboyd commented Apr 12, 2021

React-admin uses the DatePicker with the native variation and they don't expose format attribute

@wKich Can we make it work if we force users to choose the format before getting the interactor?

@wKich
Copy link
Member Author

wKich commented Apr 12, 2021

Oh, the format exists only for readonly, non-keyboard datepickers. I need to test the keyboard variant, but it seems should work

test.step(renderComponent()).assertion(Calendar({ title: "August 2014" }).exists())
.child("filters", (test) =>
test
.step(renderComponent())
Copy link
Member

Choose a reason for hiding this comment

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

Since all the of the steps in this test use the same render, maybe should make the individual actions children of the single parent that does the renderComponent() ?

Comment on lines 15 to 20
.assertion(Calendar("18 August 2014").exists())
.assertion(Calendar({ title: "August 2014" }).exists())
.assertion(Calendar({ month: "August" }).exists())
.assertion(Calendar({ year: "2014" }).exists())
.assertion(Calendar({ day: 18 }).exists())
.assertion(Calendar({ weekDay: "Mo" }).exists())
Copy link
Member

Choose a reason for hiding this comment

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

Since all the tests in this file use the same locator, I would assign it once at the top:

const calendar = Calendar("18 August 2014");

and then use the assertion form for things like filters:

assertion(calendar.has({ title: "August 2014" }));

@wKich wKich changed the title DateTimePicker Interactors WIP DateTimePicker Interactors Apr 20, 2021
@wKich wKich changed the title WIP DateTimePicker Interactors DRAFT DateTimePicker Interactors Apr 20, 2021
@cowboyd cowboyd marked this pull request as draft April 21, 2021 07:59
@cowboyd cowboyd changed the base branch from master to v4 June 10, 2021 21:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants