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

feat: date-picker #570

Merged
merged 17 commits into from
Feb 12, 2025
Merged

feat: date-picker #570

merged 17 commits into from
Feb 12, 2025

Conversation

marcjulian
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Which package are you modifying?

  • accordion
  • alert
  • alert-dialog
  • aspect-ratio
  • avatar
  • badge
  • button
  • calendar
  • card
  • checkbox
  • collapsible
  • combobox
  • command
  • context-menu
  • data-table
  • date-picker
  • dialog
  • dropdown-menu
  • hover-card
  • icon
  • input
  • label
  • menubar
  • navigation-menu
  • pagination
  • popover
  • progress
  • radio-group
  • scroll-area
  • select
  • separator
  • sheet
  • skeleton
  • slider
  • sonner
  • spinner
  • switch
  • table
  • tabs
  • textarea
  • toast
  • toggle
  • tooltip
  • typography

What is the current behavior?

Closes #112

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Implements NG_VALUE_ACCESSOR for Forms/ReactiveForms integration

Implementing BrnFormFieldControl to support FormField results in duplicate injection of HlmDatePickerComponent

@marcjulian marcjulian changed the title feat: wip date-pciker feat: wip date-picker Jan 26, 2025
@marcjulian
Copy link
Contributor Author

@goetzrobin @ashley-hunter do you have an idea how to support forms via NG_VALUE_ACCESSOR and BrnFormFieldControl for hlm-form-field without NG0200 Circular dependency in DI detected for HlmDatePickerComponent?

When I support both, they are added to the providers array

providers: [
		HLM_DATE_PICKER_VALUE_ACCESSOR,
		{
			provide: BrnFormFieldControl,
			useExisting: HlmDatePickerComponent,
		},
		provideIcons({ lucideCalendar }),
	],

But when I add formControlName to hlm-date-picker when BrnFormFieldControl is implemented the following error is thrown:

main.ts:7 ERROR RuntimeError: NG0200: Circular dependency in DI detected for HlmDatePickerComponent. Find more at https://angular.dev/errors/NG0200
    at throwCyclicDependencyError (chunk-VSQPQBST.js?v=bfac2799:2125:9)
    at getNodeInjectable (chunk-VSQPQBST.js?v=bfac2799:5072:7)
    at searchTokensOnInjector (chunk-VSQPQBST.js?v=bfac2799:5038:12)
    at lookupTokenUsingNodeInjector (chunk-VSQPQBST.js?v=bfac2799:4988:26)
    at getOrCreateInjectable (chunk-VSQPQBST.js?v=bfac2799:4929:19)
    at ɵɵdirectiveInject (chunk-VSQPQBST.js?v=bfac2799:8332:17)
    at ɵɵinject (chunk-VSQPQBST.js?v=bfac2799:2206:59)
    at factory (chunk-VSQPQBST.js?v=bfac2799:3549:23)
    at multiResolve (chunk-VSQPQBST.js?v=bfac2799:17909:17)
    at NodeInjectorFactory.multiProvidersFactoryResolver [as factory] (chunk-VSQPQBST.js?v=bfac2799:17887:10)

@ashley-hunter
Copy link
Collaborator

@goetzrobin @ashley-hunter do you have an idea how to support forms via NG_VALUE_ACCESSOR and BrnFormFieldControl for hlm-form-field without NG0200 Circular dependency in DI detected for HlmDatePickerComponent?

When I support both, they are added to the providers array

providers: [
		HLM_DATE_PICKER_VALUE_ACCESSOR,
		{
			provide: BrnFormFieldControl,
			useExisting: HlmDatePickerComponent,
		},
		provideIcons({ lucideCalendar }),
	],

But when I add formControlName to hlm-date-picker when BrnFormFieldControl is implemented the following error is thrown:

main.ts:7 ERROR RuntimeError: NG0200: Circular dependency in DI detected for HlmDatePickerComponent. Find more at https://angular.dev/errors/NG0200
    at throwCyclicDependencyError (chunk-VSQPQBST.js?v=bfac2799:2125:9)
    at getNodeInjectable (chunk-VSQPQBST.js?v=bfac2799:5072:7)
    at searchTokensOnInjector (chunk-VSQPQBST.js?v=bfac2799:5038:12)
    at lookupTokenUsingNodeInjector (chunk-VSQPQBST.js?v=bfac2799:4988:26)
    at getOrCreateInjectable (chunk-VSQPQBST.js?v=bfac2799:4929:19)
    at ɵɵdirectiveInject (chunk-VSQPQBST.js?v=bfac2799:8332:17)
    at ɵɵinject (chunk-VSQPQBST.js?v=bfac2799:2206:59)
    at factory (chunk-VSQPQBST.js?v=bfac2799:3549:23)
    at multiResolve (chunk-VSQPQBST.js?v=bfac2799:17909:17)
    at NodeInjectorFactory.multiProvidersFactoryResolver [as factory] (chunk-VSQPQBST.js?v=bfac2799:17887:10)

@goetzrobin @ashley-hunter do you have an idea how to support forms via NG_VALUE_ACCESSOR and BrnFormFieldControl for hlm-form-field without NG0200 Circular dependency in DI detected for HlmDatePickerComponent?

When I support both, they are added to the providers array

providers: [
		HLM_DATE_PICKER_VALUE_ACCESSOR,
		{
			provide: BrnFormFieldControl,
			useExisting: HlmDatePickerComponent,
		},
		provideIcons({ lucideCalendar }),
	],

But when I add formControlName to hlm-date-picker when BrnFormFieldControl is implemented the following error is thrown:

main.ts:7 ERROR RuntimeError: NG0200: Circular dependency in DI detected for HlmDatePickerComponent. Find more at https://angular.dev/errors/NG0200
    at throwCyclicDependencyError (chunk-VSQPQBST.js?v=bfac2799:2125:9)
    at getNodeInjectable (chunk-VSQPQBST.js?v=bfac2799:5072:7)
    at searchTokensOnInjector (chunk-VSQPQBST.js?v=bfac2799:5038:12)
    at lookupTokenUsingNodeInjector (chunk-VSQPQBST.js?v=bfac2799:4988:26)
    at getOrCreateInjectable (chunk-VSQPQBST.js?v=bfac2799:4929:19)
    at ɵɵdirectiveInject (chunk-VSQPQBST.js?v=bfac2799:8332:17)
    at ɵɵinject (chunk-VSQPQBST.js?v=bfac2799:2206:59)
    at factory (chunk-VSQPQBST.js?v=bfac2799:3549:23)
    at multiResolve (chunk-VSQPQBST.js?v=bfac2799:17909:17)
    at NodeInjectorFactory.multiProvidersFactoryResolver [as factory] (chunk-VSQPQBST.js?v=bfac2799:17887:10)

Unfortunately a storm here has taken out my internet connection and I only have a very limited mobile connection. It could be a few days before my connection is restored, but I'll happily look into it as soon as I am able.

@marcjulian
Copy link
Contributor Author

Unfortunately a storm here has taken out my internet connection and I only have a very limited mobile connection. It could be a few days before my connection is restored, but I'll happily look into it as soon as I am able.

No worries! Take care first!

@MerlinMoos
Copy link
Contributor

Hey, just one question. Is there a option to provide a date range picker as well like in shadcn?

@marcjulian
Copy link
Contributor Author

Date range is not yet supported by the calendar component. That need to be added first, before adding the option to the date picker component.

@marcjulian marcjulian changed the title feat: wip date-picker feat: date-picker Feb 4, 2025
@marcjulian marcjulian marked this pull request as ready for review February 4, 2025 11:17
@marcjulian
Copy link
Contributor Author

@goetzrobin @ashley-hunter date picker component is ready for review. It integrates with Forms/ReactiveForms.

One thing thats missing is additional support for FormField via BrnFormFieldControl, this can be added in a follow up PR. Not sure how to provide BrnFormFieldControl together with HLM_DATE_PICKER_VALUE_ACCESSOR. I guess will need to solve it once and can use it for other components too like combobox.

Copy link
Collaborator

@ashley-hunter ashley-hunter left a comment

Choose a reason for hiding this comment

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

Thank you! Great work! Just noticed a few small things, apart from that looks really good!


public readonly userClass = input<ClassValue>('', { alias: 'class' });
protected readonly _computedClass = computed(() =>
hlm(
Copy link
Collaborator

Choose a reason for hiding this comment

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

There appears to be a border and a box shadow in our implementation which differs from the shadcn implementation:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I checked the shadcn/ui styles and it looks like the calendar only receives border styles when used in line. I override the border styles for the date-picker calendar, thus only the styles from the popover content are applied.

Comment on lines 55 to 56
nxCode="npx nx g @spartan-ng/cli:ui date-picker"
ngCode="ng g @spartan-ng/cli:ui date-picker"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the CLI ui generator has been updated to support this component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes totally forgot about that! I added it to the generator and updated the primitive dependencies as well

Copy link
Collaborator

@ashley-hunter ashley-hunter 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 to me, thanks!

@JoshuaMorley
Copy link

Awesome work! I just tried to use the library and noticed this was missing. Great to see it so close to making it in 🥇

@goetzrobin goetzrobin merged commit 9512b11 into spartan-ng:main Feb 12, 2025
6 checks passed
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.

RFC: Date Picker
5 participants