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

fix(geo): Layer color picker do not change the fill/stroke color #1509

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

pelord
Copy link
Member

@pelord pelord commented Nov 20, 2023

Please check if the PR fulfills these requirements

What is the current behavior? (You can also link to an open issue here)
related to issue #1508

What is the new behavior?
Fix it?

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications:

Other information:

@pelord pelord requested a review from alecarn November 20, 2023 19:36
* The emitted value represent the current chosen color in the picker
* The value is emitted by default in RGBA, or based on the input 'outputFormat' value.
*/
@Output() colorChange: EventEmitter<string> = new EventEmitter<string>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cette composante est une variant d'un FormField elle fonctionne avec le ReactiveForm d'Angular. Lorsqu'on lui passe un FormControl, on peut s'abonner au changement au niveau du FormControl. Voir l'exemple dans le prochain commentaire

Copy link
Member Author

Choose a reason for hiding this comment

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

Oui sauf que dans ce cas la, pour setter le form, il faut cliquer sur ok ou appliquer, donc le changement n'est pas dynamique.

@@ -14,6 +14,7 @@ <h2 mat-dialog-title class="mat-typography">
</span>
<igo-color-picker-form-field
formControlName="fill"
(colorChange)="setLayerFillColor($event)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

On pourrait avoir les changements de valeurs ainsi dans le Typescript de la composante:

Suggested change
(colorChange)="setLayerFillColor($event)"
this.form
.get('fill')
.valueChanges.subscribe((color) => this.handleFillColor(color));

Copy link
Member Author

Choose a reason for hiding this comment

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

Il faut cliquer sur Appliquer/OK pour récupérer la couleur

Copy link
Member Author

Choose a reason for hiding this comment

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

chrome_2023-11-20_16-24-49

Copy link
Member Author

Choose a reason for hiding this comment

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

Vraiment plus convivial pour choisir une couleur, surtout si des superpositions/conflits visuel de couleur sont possible.

@alecarn alecarn merged commit 424214e into next Nov 20, 2023
4 checks passed
@alecarn alecarn deleted the layer-color-picker-1508 branch November 20, 2023 21:32
@alecarn alecarn added this to the v16.1.0 milestone Nov 20, 2023
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.

None yet

2 participants