Skip to content

Conversation

@mfori
Copy link
Member

@mfori mfori commented Sep 25, 2024

Add new datepicker allowAbsolute and allowRelative options dateType property to input schema specification.
This is based on this issue: https://github.com/apify/apify-core/issues/15662 and part of

This one will be merged as the last one.

Screenshot 2024-11-01 at 9 45 29

@mfori mfori added documentation Improvements or additions to documentation. t-console Issues with this label are in the ownership of the console team. labels Sep 25, 2024
@mfori mfori requested review from TC-MO and gippy September 25, 2024 13:32
@mfori mfori self-assigned this Sep 25, 2024
@github-actions github-actions bot added this to the 99th sprint - Console team milestone Sep 25, 2024
Copy link
Member

@drobnikj drobnikj left a comment

Choose a reason for hiding this comment

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

Thing I found in other PR.

@mfori mfori requested a review from drobnikj September 25, 2024 15:02
@drobnikj
Copy link
Member

Just note to do not fofget, the params were changes from is* to allow*

@mfori mfori changed the title docs: add datepicker isAbsolute/isRelative options docs: add datepicker allowAbsolute/allowRelative options Sep 27, 2024
Copy link
Contributor

@TC-MO TC-MO left a comment

Choose a reason for hiding this comment

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

LGTM

mfori added a commit to apify/apify-shared-js that referenced this pull request Sep 30, 2024
… and validation (#477)

This PR introduce new input_schema properties `allowAbsolute` and
`allowRelative` used when the `editor` property is set to `datepicker`:
- the `schema.json` is updated in the way, that properties
`allowAbsolute` and `allowRelative` are valid only when `editor` is
`datepicker`, `allowAbsolute` is considered as default and any of
`allowAbsolute` or `allowRelative` is true
- value format validation
- unit tests for schema validation and value validation
- New datepicker with support for absolute and relative dates PR:
apify/apify-core#17465
- Docs PR: apify/apify-docs#1227

---------

Co-authored-by: Martin Adámek <[email protected]>
@mfori mfori requested review from mtrunkat and netmilk October 7, 2024 14:17
| `enumTitles` | [String] | No | Titles for the `enum` keys described. |
| `nullable` | Boolean | No | Specifies whether `null` <br/>is an allowed value. |
| `isSecret` | Boolean | No | Specifies whether the input field<br />will be stored encrypted.<br />Only available <br />with `textfield` and `textarea` editors. |
| `allowAbsolute` | Boolean | No | Used only with `datepicker` editor for absolute date value picker in form `YYYY-MM-DD`. This defaults to `true`. |
Copy link
Member

Choose a reason for hiding this comment

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

The properties are fine, but I don't understand what they mean well. Perhaps there should be better explanation. Is it:

  • If I enable allow absolute then I am able to pick certain date
  • If I enable allow relative then I can type "3 months"??
    But what is the returned value is what? Always a date in ISO format?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, the docs could be improved. The value is always string.

If you set allowAbsolute true (which is the default value, so you do not have to set it), then the datepicker allows user to select a date from a calendar and the final value is stored in YYYY-MM-DD format and also validated like this.

If you pick allowRelative true (which is not the default value), then the datepicker allows user to enter a value in a format +- <number> <unit> and the final string value in the field is validated against this format.

If you have both set to true, then in the UI you can choose which format you want to use.
It's then up to the actor to correctly parse the value, but it will be validated in UI and in API.

The reason for this logic is that store team needs to have a single field that can be switched between absolute date like 2024-10-08 and relative option - 3 days. For example when you want comments on a post from a specific day or for last 3 days relative to when the actor started. They do not want to use two fields because that can then cause confusion.

They currently work around this by having a single string field with no validation (or regex). This should allow them to validate the values correctly and help users enter the correct values in a standardized format.

By default if you do not set any of these two properties, the datepicker will behave exactly the same as before and only show a single field with calendar in popover.

Copy link
Member

Choose a reason for hiding this comment

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

I think the allowRelative explanation is good, but the allowAbsolute can be improved. Also could maybe show an example like we do it for other more complicated fields.

Copy link
Member

Choose a reason for hiding this comment

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

So, in this case, we should have 2 different editors or "resources types". You should not have varying outputs from a single component.

Copy link
Member

Choose a reason for hiding this comment

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

Like one function should always return either date object or relative string but not both.

Copy link
Member

@gippy gippy Oct 9, 2024

Choose a reason for hiding this comment

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

But the main requirement from store team is to have a single field, so it's one editor, which can return two different resource types based on users choice, but not both at the same time. Author of the actor can choose which options to allow

Copy link
Member

Choose a reason for hiding this comment

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

Ok, that makes sense then.

mfori added 2 commits October 11, 2024 07:34
# Conflicts:
#	sources/platform/actors/development/actor_definition/input_schema/specification.md
@mfori
Copy link
Member Author

mfori commented Oct 11, 2024

Added also example as we have with some other editors:

Screenshot 2024-10-11 at 8 11 47

@mfori mfori requested review from TC-MO and mtrunkat October 11, 2024 06:13
Copy link
Contributor

@TC-MO TC-MO left a comment

Choose a reason for hiding this comment

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

few rewrites for clarity & consistency with the rest of the docs

| `nullable` | Boolean | No | Specifies whether `null` <br/>is an allowed value. |
| `isSecret` | Boolean | No | Specifies whether the input field<br />will be stored encrypted.<br />Only available <br />with `textfield` and `textarea` editors. |
| `allowAbsolute` | Boolean | No | Used only with `datepicker` editor for absolute date value picker in form `YYYY-MM-DD`. This defaults to `true`. |
| `allowRelative` | Boolean | No | Used only with `datepicker` editor for relative input in form `+/- {number} {unit}`.<br/>The value passed to Actor input is in plain text e.g. `"+ 3 weeks"` and it's up to Actor author to parse it.<br/>Supported units are: days, weeks, months and years.<br/>If only relative input is required, `allowAbsolute` needs to be explicitly set to `false`, otherwise both formats are supported. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `allowRelative` | Boolean | No | Used only with `datepicker` editor for relative input in form `+/- {number} {unit}`.<br/>The value passed to Actor input is in plain text e.g. `"+ 3 weeks"` and it's up to Actor author to parse it.<br/>Supported units are: days, weeks, months and years.<br/>If only relative input is required, `allowAbsolute` needs to be explicitly set to `false`, otherwise both formats are supported. |
| `allowRelative` | Boolean | No | Enables relative date input in <br/>`+/- {number} {unit}` format. <br/>Supported units are: days, weeks, months, years. <br/><br/>The input is passed to the Actor as plain text (e.g., "+3 weeks"). Set `allowAbsolute` to `false` to allow only relative input, otherwise both formats are supported. Only available with `datepicker` editor. |

Copy link
Member

Choose a reason for hiding this comment

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

Can we also recommend here a library we use to parse this? It's docs for devs so let's give them some hints.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added regex that can be used to parse the relative value:

Screenshot 2024-10-13 at 0 17 59

}
```

Rendered input for `absolute_date` property. See that by default, the date picker allows selecting only an absolute date in the form `YYYY-MM-DD`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Rendered input for `absolute_date` property. See that by default, the date picker allows selecting only an absolute date in the form `YYYY-MM-DD`:
The `absolute_date` property renders a date picker that allows selection of a specific date in the format `YYYY-MM-DD`.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I'd stress here that this is the value returned by the field. See comment bellow.


![Apify Actor input schema - country input](./images/input-schema-date-absolute.png)

Rendered input for `relative_date` property in form of `+/- {number} {unit}`. (The `allowAbsolute` is explicitly set to `false`):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Rendered input for `relative_date` property in form of `+/- {number} {unit}`. (The `allowAbsolute` is explicitly set to `false`):
The `relative_date` property renders an input field that accepts dates in the `+/- {number} {unit}`. nthe `allowAbsolute` parameter is set to `false` to restrict input to relative dates only.

Copy link
Member

Choose a reason for hiding this comment

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

IMHO, to make this clear, we should write that

...renders an input field that enables the user to choose the relative date and returns the value in the format +/- {number} {unit}, for example, + 2 days.

This is a documentation for Actor developers so we need to focus on what is returned by the field which is what they work with.

@mfori
Copy link
Member Author

mfori commented Oct 12, 2024

Screenshot 2024-10-13 at 0 20 44

Screenshot 2024-10-13 at 0 21 09

Screenshot 2024-10-13 at 0 20 27

@mfori mfori requested a review from TC-MO October 12, 2024 22:21
Copy link
Member

@jancurn jancurn left a comment

Choose a reason for hiding this comment

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

Looks good, thanks, just last small changes

@mfori
Copy link
Member Author

mfori commented Oct 31, 2024

We have changed allowAbsolute/allowRelative properties to single property dateType with options absolute, relative and absoluteOrRelative as proposed here: https://apify.slack.com/archives/C010Q0FBYG3/p1730210649660309?thread_ts=1728907212.870409&cid=C010Q0FBYG3

So I will ask you for one more review round 🙏 😁

Copy link
Member

@jancurn jancurn left a comment

Choose a reason for hiding this comment

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

Great stuff, thank you

@mfori mfori requested a review from TC-MO October 31, 2024 12:37
@mfori mfori changed the title docs: add datepicker allowAbsolute/allowRelative options docs: add datepicker dateType property to input schema Nov 1, 2024
mfori added a commit to apify/apify-shared-js that referenced this pull request Nov 5, 2024
This PR replace `allowAbsolute`/`allowRelative` boolean properties for
datepicker with single `dateType` property that accepts values
`absolute`, `relative` and `absoluteOrRelative` based on:
- apify/apify-docs#1227
-
https://apify.slack.com/archives/C010Q0FBYG3/p1730210649660309?thread_ts=1728907212.870409&cid=C010Q0FBYG3

`dateType` property:
- is optional
- is valid only when `editor` is `datepicker`
- is only for ui input representation, no validation is executed based
on this property
@mfori mfori merged commit b923891 into master Nov 14, 2024
11 checks passed
@mfori mfori deleted the feat/datepicker-absolute-relative branch November 14, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation. t-console Issues with this label are in the ownership of the console team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants