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

Adding colors to color palette #3215

Open
yoSachinkr opened this issue Sep 13, 2023 · 10 comments · May be fixed by #3334
Open

Adding colors to color palette #3215

yoSachinkr opened this issue Sep 13, 2023 · 10 comments · May be fixed by #3334
Assignees

Comments

@yoSachinkr
Copy link

Describe the bug
With the current changes in eyedropper tool, it now sets the fill color to the picked color. But when you try to add color to the color palette, it adds the outline color into the palette, which causes inconvenience.

Expected behavior
Color palette should add fill color instead of outline color.

Screenshots
image

System information:

  • OS: Windows 10
  • OS version: 64 bit 21h2
@rodolforg
Copy link
Contributor

@morevnaproject do you agree?

@yoSachinkr
Copy link
Author

@rodolforg Since the change, when you pick a color with the eyedropper tool, the outline color never changes now, it's the fill color that changes. That means "adding outline color to the palette" will only add the same color to the palette again and again, which obviously is not intended.
Earlier, when eyedropper tool used to pick the outline color, "adding outline color to the palette" would have made sense. Not anymore now.

@ImDoubD
Copy link

ImDoubD commented Nov 16, 2023

Hello, Duke this side. I find this issue interesting and want to resolve it. Can you assign this issue to me?
Also I am new to Open Source Contribution so any guidance will be of great help.

@ice0
Copy link
Collaborator

ice0 commented Nov 17, 2023

Hello, Duke this side. I find this issue interesting and want to resolve it. Can you assign this issue to me? Also I am new to Open Source Contribution so any guidance will be of great help.

Hi, Duke!

Sure, assigned it to you.
First, you need to clone project locally and install build environment.
To do that you can check this documentation:
https://synfig-docs-dev.readthedocs.io/en/latest/community/contribution%20guidelines.html

Currently Linux/macOS is recommended for development (you can easily run Linux in VM under Windows).

P.S. Also, if you find any inaccuracies in the documentation or installation process, you can contribute to the documentation too :)

@ImDoubD
Copy link

ImDoubD commented Nov 17, 2023

Hello, Duke this side. I find this issue interesting and want to resolve it. Can you assign this issue to me? Also I am new to Open Source Contribution so any guidance will be of great help.

Hi, Duke!

Sure, assigned it to you. First, you need to clone project locally and install build environment. To do that you can check this documentation: https://synfig-docs-dev.readthedocs.io/en/latest/community/contribution%20guidelines.html

Currently Linux/macOS is recommended for development (you can easily run Linux in VM under Windows).

P.S. Also, if you find any inaccuracies in the documentation or installation process, you can contribute to the documentation too :)

Can you tell me in which file contains the color palette code.
here is my discord link: https://discord.gg/NADgx2AJ
I wish to connect with you regarding this contribution for further clarification if possible.
Thanks

@rodolforg
Copy link
Contributor

Can you tell me in which file contains the color palette code.

GUI: synfig-studio/src/gui/modules/mod_palette/
lib: synfig-core/src/synfig/palette.{h,cpp}

@KartikWatts
Copy link
Contributor

KartikWatts commented Jan 24, 2024

@rodolforg @ice0 Hi, as I felt this issue being inactive, I have tried to fix this locally already, if it is kindly possible, can it be please assigned to me, will raise a PR to fix this. Being one of the software I regularly use myself, it would be a great opportunity to be able to contribute. Attaching local patch functionality:

synfig_fill_palette

Regards.

Also, while testing this I came across a minor issue (in my opinion), a user can select the same color multiple times in the palette, if this needs to be fixed, please let me know, I may raise a separate issue and PR for this fix in that case, or as instructed. Would love to resolve this as well. Looking forward, thanks a lot.

@rodolforg rodolforg assigned KartikWatts and unassigned ImDoubD Jan 25, 2024
@rodolforg
Copy link
Contributor

a user can select the same color multiple times in the palette, if this needs to be fixed, please let me know, I may raise a separate issue and PR for this fix in that case,

You can warn user that a equal color is already in palette and ask for confirmation if he/she really want to duplicate it.

I don't know if you need to create a new issue; I guess you can directly create another PR if you want.

@KartikWatts
Copy link
Contributor

@rodolforg Raised the PR for the issue mentioned.

You can warn user that a equal color is already in palette and ask for confirmation if he/she really want to duplicate it.

Yeah, that would be great. I'll work on this and raise another PR for the functionality. Regards.

@KartikWatts
Copy link
Contributor

@rodolforg Got delayed in implementing the enhancement. I have just implemented it and I'm raising the PR.
The UIInterface::confirmation dialog will be triggered when the User attempts to add a color already present in the palette. Will attach more details in the PR.

Attaching GIF for reference:

synfig_color_duplicate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants