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

Some pastel love #4

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Some pastel love #4

wants to merge 8 commits into from

Conversation

boun
Copy link

@boun boun commented Feb 28, 2013

Hi,

this branch does two things:

** First (commits a3a3787, e038c3f, cleaned up in 6b3b5d5, 03a54b6 ) **

Adds color settings to all notes and styles the ActionBar. It includes look&feel, some code & pngs from MIUI notes, which is also Apache licened:

https://github.com/ebraminio/miuinotes

Side Effects: Introduces new DB format (color column).

** Second (a047838) **

Wraps the note_editor.xml into a scrollview, which vastly improves scrolling in larger notes.

** Rest **

Cleans up broken indenting (courtesy eclipse) and adapts some settings for my froyo device.

Tested: 4.2.2 (JB) and 2.3.? (FR)

TODO:

It ignores the shipped theming system, which I dont care that much ;)
Mention the miui graphics in the EULA
Maybe set the theming only in Code if SDK > 12 (14?)

Comments?

Screen Shot 2013-02-21 at 8 22 34 PM

@aperomsik
Copy link
Member

Friedger, a later commit in this stack reverted the minSdkVersion change. I agree it would have been less confusing if the commit history could have been cleaned up a bit before submitting the pull request though. http://sethrobertson.github.com/GitPostProduction/gpp.html

Also, if I understand "It ignores the shipped theming system, which I dont care that much" correctly, the OI themes could not be used with this patch set applied. If that's true, of course it's unacceptable to us.

Tempted to cherry-pick just the scrollview commit when I have some time to test it.

@aperomsik
Copy link
Member

... except I missed the darkPink bit.

@boun
Copy link
Author

boun commented Mar 3, 2013

@aperomsik i will cleanup next time.

Ignore means just ignore. You can still apply themes. The comment basically means, that I added a new db column to store background colors, which I - perhaps - could have somehow stored on top of the existing theming system.

If you perfer to go with the scrollview commit, go for it, scrolling needs some love too ;)

@aperomsik
Copy link
Member

If you submit the scrollview change separately, but without color changes, that is more likely to get accepted faster.

Then it seems the next step would be to add a preference for using the note's theme in the note list view. It should affect the background and foreground, and perhaps also other theme attributes like fonts. That could be a second pull request.

Then one possible next step would be a way to add some theme-specific settings in the theme selector. Right now the "Basic colors" theme has three variations; but instead of showing up in the list as three separate themes perhaps it could be made to show as one theme with an option to choose from three color schemes. The pastel could be a simlar new theme which also has an option to choose which color from the list of pastel colors you want to support.

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.

3 participants