goetzrobin / spartan

Cutting-edge tools powering Angular full-stack development.
https://spartan.ng
MIT License
1.42k stars 152 forks source link

Dialog event (stateChanged) not emit values #100

Closed rpacheco124 closed 9 months ago

rpacheco124 commented 9 months ago

Please provide the environment you discovered this bug in.

Angular CLI: 17.0.0 Node: 21.4.0 Package Manager: npm 10.2.4 OS: win32 x64

{
  .
  .
  .
  .
  "dependencies": {
    "@angular/animations": "^17.0.0",
    "@angular/cdk": "17.0.0",
    "@angular/common": "^17.0.0",
    "@angular/compiler": "^17.0.0",
    "@angular/core": "^17.0.0",
    "@angular/forms": "^17.0.0",
    "@angular/platform-browser": "^17.0.0",
    "@angular/platform-browser-dynamic": "^17.0.0",
    "@angular/router": "^17.0.0",
    "@ng-icons/core": "^25.1.0",
    "@ng-icons/radix-icons": "^26.3.0",
    "@ngneat/cmdk": "^2.0.0",
    "@spartan-ng/ui-command-brain": "0.0.1-alpha.319",
    "@spartan-ng/ui-core": "^0.0.1-alpha.319",
    "@spartan-ng/ui-dialog-brain": "0.0.1-alpha.319",
    "@spartan-ng/ui-popover-brain": "0.0.1-alpha.319",
    "class-variance-authority": "^0.6.0",
    "clsx": "^1.2.1",
    "rxjs": "~7.8.0",
    "tslib": "^2.3.0",
    "zone.js": "~0.14.2"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "^17.0.7",
    "@angular/cli": "^17.0.7",
    "@angular/compiler-cli": "^17.0.0",
    "@spartan-ng/cli": "^0.0.1-alpha.331",
    "@types/jasmine": "~5.1.0",
    "autoprefixer": "^10.4.16",
    "jasmine-core": "~5.1.0",
    "karma": "~6.4.0",
    "karma-chrome-launcher": "~3.2.0",
    "karma-coverage": "~2.2.0",
    "karma-jasmine": "~5.1.0",
    "karma-jasmine-html-reporter": "~2.1.0",
    "postcss": "^8.4.32",
    "tailwind-merge": "^1.14.0",
    "tailwindcss": "^3.4.0",
    "tailwindcss-animate": "^1.0.6",
    "typescript": "~5.2.2"
  }
}

Which area/package is the issue in?

command and popover

Description

I am trying to replicate the code published at spartan-ui/combobox in a new project. These are the steps I followed:

ng new spartan-ui-combobox-example

// Install Tailwind 

// Install Spartan UI

npm i -D @spartan-ng/cli

npm i @spartan-ng/ui-core

ng g @spartan-ng/cli:ui-theme

// Now I follow the steps to install the primitives needed for the combobox
ng g @spartan-ng/cli:ui command

ng g @spartan-ng/cli:ui popover

NOTE HERE -> the command to generate the command primitive does not install the "@ngneat/cmdk" dependency, and the "@ng-icons/radix-icons" dependency is also not installed. I had to manually install these dependencies.

I copy and paste the example code for the combobox, and everything seems to work fine. However, when I click on some of the combobox options, it does not close. To close it, I have to click outside the combobox.

spartan-ui-bug-dialog

Please provide the exception or error you saw

No response

Other information

No response

I would be willing to submit a PR to fix this issue

ajitzero commented 9 months ago

The current behavior is expected behaviour for a combobox, especially when selecting multiple items or searching for something and hitting "enter", we don't want it to be closed.

Your point makes sense in a single-select scenario and would be good to have as a directive/attribute here, like [comboboxSettings]="{ single: true }" or [closeOnSelect] - That way we don't need to track emitted event to do anything manually, and the actual emission (if needed), can be taken from the FormControl's valueChanges stream (or with ngModelChanges, take your pick).

What do you think?

rpacheco124 commented 9 months ago

Hi @ajitzero

I understand your point, but this issue is because I am trying to replicate the code that is in the documentation, in the documentation the combobox closes when I select an option.

I'm not sure if in the first version the combobox will support multiple options, we would have to see how shadcn has it, if it has a special component for multiselect or if a combobox is used.

I like your idea, thanks for commenting.

goetzrobin commented 9 months ago

This will be part of the next release!

goetzrobin commented 9 months ago

Issue was addressed as part of releases: cli 0.0.1-alpha.334 & ui 0.0.1-alpha.321