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

Accent color #138

Merged
merged 51 commits into from
Apr 22, 2021
Merged

Accent color #138

merged 51 commits into from
Apr 22, 2021

Conversation

meisenzahl
Copy link
Member

@meisenzahl
Copy link
Member Author

The accent color is now set as AccentColor. @cassidyjames do we want to possibly prefix that with io.elementary.?

@cassidyjames
Copy link
Contributor

@meisenzahl I'm not super familiar with how XMP in EXIF works, but it sounds like it would be good to RDNN prefix it to avoid potential collisions?

@meisenzahl
Copy link
Member Author

@cassidyjames Yes, exactly. Feel free to suggest what the attribute should be called.

@cassidyjames
Copy link
Contributor

cassidyjames commented Apr 5, 2021

@meisenzahl io.elementary.AccentColor seems fine to me. Could we ensure we're matching this mapping, though? I think we're also generally moving to only use the cutesy food names in user strings and just use more standard color names anywhere for developers.

elementary/default-settings@17d737e

@meisenzahl
Copy link
Member Author

@cassidyjames I'll take a look to see if we can ensure we're matching this mapping.

What do you think about a command line tool that makes sure that when you add a new image attributes like author and accent color are set?

I can refactor to use the following colors:

0 = No Preference (currently assumes Blue)
1 = Red
2 = Orange
3 = Yellow
4 = Green
5 = Mint
6 = Blue
7 = Purple
8 = Pink
9 = Brown
10 = Gray

@cassidyjames
Copy link
Contributor

@meisenzahl I would be open to is as long as it's simple enough to use and maintain. :)

@meisenzahl
Copy link
Member Author

@cassidyjames ready for review. I hope I have set all colors correctly now :)

@meisenzahl meisenzahl marked this pull request as ready for review April 6, 2021 18:22
@cassidyjames
Copy link
Contributor

@meisenzahl okay I talked this through with Dan to make sure this was a good approach, and we agree EXIF seems like a good fit. But what's the need for the command-line app? It seems that we have to copy-paste some commands from the README either way, so this seems like it might be over-complicated compared to just using exiftool directly.

@meisenzahl
Copy link
Member Author

meisenzahl commented Apr 8, 2021

@cassidyjames with the command line application you can set the EXIF information, but you can also lint the files. The cli tool also checks if the provided accent color is a valid value.

I was not able to use exif to extract the custom io.elementary.AccentColor metadata. Maybe that works too, but I am not sure...

@cassidyjames
Copy link
Contributor

@meisenzahl gotcha. I think for now, let's split the command line tool off into its own branch so we can discuss/debate it there—but I'm fine with the inclusion of the EXIF data here.

@meisenzahl
Copy link
Member Author

I have cleaned up the PR so far. However, I am not sure how to write the exiftool commands and create the configuration file. Using RDNN makes things a bit more complicated 😅️

@cassidyjames shall we discuss how to proceed with adding and verifying the metadata in #140?

@cassidyjames cassidyjames merged commit bb043b0 into master Apr 22, 2021
@cassidyjames cassidyjames deleted the accent-color branch April 22, 2021 23:02
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