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

Position new palette colors at bottom, and renaming immediately #1395

Merged
merged 7 commits into from Jul 20, 2020

Conversation

davidlamhauge
Copy link
Contributor

@davidlamhauge davidlamhauge commented Jul 10, 2020

This fixes a 'bug' that has often bothered me.

  1. New colors were placed in front of the selected color, and that made sense before the colors could be dragged and dropped. I always expect to find the new colors at the bottom of the palette, because they logically should be appended to the palette. This PR places new colors at the bottom of the list.
  2. The reason for having names for the colors is to use them. If we don't use the names, we might as well just have them numbered. My colors get names like 'Lisa skin', 'Lisa eyes', 'Lisa Hair' etc, and not 'Magenta', 'Vivid Reddish Purple', or some of the other suggested names. For that reason, I've put the rename color dialog up front, so you're naming your color when you add it. This is more logical, and if you indeed want the name to be 'Vivid Reddish Purple', then just press enter.

@chchwy
Copy link
Member

chchwy commented Jul 19, 2020

I made a change for this PR.

Rather than popping up a rename dialog. I make the new color editable right after clicking "add".
image

What do you think? @davidlamhauge @candyface @scribblemaniac

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Jul 19, 2020

Great idea. Sorry I didn't think of that.

@MrStevns
Copy link
Member

MrStevns commented Jul 19, 2020

That's exactly how I would improve it based on my comment in Discord made the other day, I approve of such a change.

@chchwy
Copy link
Member

chchwy commented Jul 19, 2020

@davidlamhauge I made a PR in your repo
davidlamhauge#13

@scribblemaniac
Copy link
Member

scribblemaniac commented Jul 19, 2020

Do both views scroll down to the new color when added? If not, they should, especially the list view so you can see the newly added/editable color.

Make the newly added color editable
@chchwy
Copy link
Member

chchwy commented Jul 20, 2020

Yes it does scroll down to the new color.

@chchwy chchwy merged commit d9c1feb into pencil2d:master Jul 20, 2020
1 check passed
@Jose-Moreno Jose-Moreno added this to the 0.6.5 milestone Jul 30, 2020
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

5 participants