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

#1174 More consistent palette import behavior #1262

Merged
merged 10 commits into from Jan 6, 2020

Conversation

davidlamhauge
Copy link
Contributor

@davidlamhauge davidlamhauge commented Sep 11, 2019

David_Pencil2d_20190911_20_20_22
This PR addresses the inconsistent behavior on opening, importing, saving and exporting palettes.
As we agreed, this is how it goes:

  1. Opening a palette, replaces the existing palette with a new one. If at least one color is in use on a vector layer, a warning is given. Only one warning.
  2. Importing a palette, appends the new palette to the old one.
  3. Saving a palette can be done to the Pencil2D xml format.
  4. Exporting a palette can be done to GPL, XML, and other formats as soon as the formats are implemented.
    I hope I've covered all (or most) issues we've talked about.
    This closes Inconsistent palette import behavior #1174

@scribblemaniac scribblemaniac self-requested a review Sep 11, 2019
@scribblemaniac scribblemaniac added Color Palette Enhancement UI Related to the visual appearance of the program 🔹 Minor PR (only one reviewer required) labels Sep 13, 2019
@MrStevns MrStevns added this to the 0.6.5 milestone Nov 10, 2019
@MrStevns MrStevns self-requested a review Nov 10, 2019
Copy link
Member

@MrStevns MrStevns left a comment

I think the export UI should be simplified, the functionality itself looks fine.

Let's keep it at only one export palette action and let the user decide which extension they want to use.

app/src/mainwindow2.cpp Outdated Show resolved Hide resolved
app/ui/mainwindow2.ui Outdated Show resolved Hide resolved
core_lib/src/structure/object.cpp Outdated Show resolved Hide resolved
@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Nov 11, 2019

I have merged the newest Master branch into this branch, and fixed the typs.
I also removed the option to choose whether to export as Pencil2D or GIMP format. It works, but on my Ubuntu you must add suffix yourself.
I'm too tired to fix it today... 😪

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Nov 14, 2019

Now, there is always an extension.

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Nov 24, 2019

It again saves without file extension in Linux (Ubuntu 18.04).
Otherwise it works fine @candyface

@MrStevns
Copy link
Member

MrStevns commented Nov 24, 2019

The save without extension on linux should be fixed via #1299

The issue you mention stems from gtk3 not respecting its filter selector, so if no extension is given, then the file will be saved with no extension.

@MrStevns
Copy link
Member

MrStevns commented Jan 2, 2020

I'd say this is ready to be reviewed again whenever the remaining conflicts has been fixed.

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Jan 2, 2020

Conflicts has been fixed.

@@ -63,6 +63,8 @@ GNU General Public License for more details.
#define PFF_DEFAULT_PALETTE_EXT \
QString(".xml")

#define PFF_PALETTE_EXT_FILTER \
Copy link
Member

@MrStevns MrStevns Jan 5, 2020

Choose a reason for hiding this comment

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

This macro should be removed, It's redundant since it is defined above.

Copy link
Contributor Author

@davidlamhauge davidlamhauge Jan 6, 2020

Choose a reason for hiding this comment

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

Removed

Copy link
Member

@MrStevns MrStevns left a comment

LGTM, thanks @davidlamhauge 👍

@MrStevns MrStevns merged commit 9ad6c32 into pencil2d:master Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Color Palette Enhancement 🔹 Minor PR (only one reviewer required) UI Related to the visual appearance of the program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent palette import behavior
3 participants