Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AuDatePicker] Suggestions / issues #90

Closed
Windvis opened this issue Apr 21, 2021 · 0 comments
Closed

[AuDatePicker] Suggestions / issues #90

Windvis opened this issue Apr 21, 2021 · 0 comments

Comments

@Windvis
Copy link
Contributor

Windvis commented Apr 21, 2021

I'm integrating the AuDatePicker component in GN and I noticed some things that can be improved:

Integration issues

At the moment the <duet-date-picker> component doesn't actually work in other projects. The html is rendered, but the web component logic isn't being registered / executed. This addon uses ember-cli-stencil to make stencil web components work automagically, but it is installed as a devDependency which means it's not installed in the host app. It might be possible to simply move it to the dependencies but it would have to be tested.

An alternative could be to create an initializer which does the initialization. The code would look similar to this commit. I think this approach is better since the ember-cli-stencil addon does some very dynamic things which I suspect won't be compatible with Embroider. Since the manual version is so simple I think it makes sense to just sidestep that problem altogether.

Suggested API changes:

  • The AuDatePicker expects an ISO 8601 formatted date as a value because it is passed through as-is to the <duet-date-picker> component. Since Ember supports arguments of different types than string, I think it would make more sense to support Date instances as well (or only). That way the consuming app doesn't have to convert dates to strings manually.
  • The same goes for the value that gets passed into the callback function. Returning a Date instance to update a date or undefined / null to clear it would be a nicer DX.

Issues:

  • Typing in the input field doesn't update the calendar when opened. This does work on the <duet-date-picker> demo page although I haven't looked into the details yet.
  • Clearing the input field throws an exception because the callBackParent action doesn't handle that yet.
  • Translations are set when the component is created so to change the translations the component needs to be destroyed first. I think it would be better to convert this logic into a getter so it gets updated if the localization argument changes. Not super important since it will probably not be a problem in practice, but it would make the component more solid.

PR for reference: lblod/frontend-gelinkt-notuleren#89

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

No branches or pull requests

2 participants