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

Added Present's background color as an option to the Preferences window. #2568

Merged
merged 5 commits into from Jul 25, 2014

Conversation

Projects
None yet
3 participants
@joelmoniz
Copy link
Member

joelmoniz commented Jun 7, 2014

Hey,

In these few commits, I've added an option for the user to conveniently choose a background color for Present from the Preferences window. This can be done in 2 ways:

  1. By clicking on a color preview box, which automatically pops up a ColorChooser.
  2. By typing a hex color into the Text Field.

The text field has been designed to accept a hex color of the form #xxxxxx and xxxxxx, (x- 0 to F), and auto-previews the color once the user enters a valid hex color.

A Cancel button has also been added to the ColorChooser, because it seemed like a nice feature to have for both the Color Selector tool, as well as for this particular application.

Thanks

joelmoniz added some commits May 31, 2014

Add 'link' between pref window and file for presnt
Sets present.bgcolor on clicking OK, sets color in Preferences window from
Preferences.txt on opening the window.
Changes as to how the user selects Present bckgrnd
Clicking on the color preview brings up a color selector; Inputting a hex color automatically previews it
Shed a lot of weight, some UI changes
Rm CustomColorChooser, add Cancel to ColorChooser,
Formatting changes,
Removed unnecessary listener,
Inerchange preview color and text field.
Done adding Present bckgrnd color to Prefs window
Cursor to hand on mouseover & new border for color preview
@benfry

This comment has been minimized.

Copy link
Member

benfry commented Jun 7, 2014

Whoa, this is much more complicated than necessary. The color choosing is already implemented in the Export to Application dialog, it should only be a couple lines of code to add that to the Preferences window as well.

@benfry benfry closed this Jun 7, 2014

@fjenett

This comment has been minimized.

Copy link
Member

fjenett commented Jun 8, 2014

Ben, it's a bit unclear to us how we should proceed. I propose:

  • make child class JavaEditor.ColorPreference into a full class so i can be reused
  • replace in JavaEditor
  • add ColorPreference to preferences dialog where we had our custom color chooser button
  • remove text input field from preferences dialog (as it is avail through color chooser dialog)
  • keep the "cancel" button in the color chooser dialog?

OK?

@benfry

This comment has been minimized.

Copy link
Member

benfry commented Jun 8, 2014

Perhaps I'm just confused by the commit. It looks like an entire new color chooser class has been added and removed. Or maybe there's a line endings problem that makes this look much larger?

Check out the links for those commits... Maybe it's just a matter of re-doing the pull request so that I can see what's actually going on?

@benfry benfry reopened this Jun 8, 2014

@fjenett

This comment has been minimized.

Copy link
Member

fjenett commented Jun 8, 2014

Oh, you are looking at every single commit. That is not needed, just click "Files changed" at the top, that gives you the changes that will be applied in relation to the current code.

And yes, Joel initially duplicated the color chooser and i asked him to reuse the one we already had instead. So it is actually not that much change in total.

We can even slim it down further and make the style be concise if we make ColorPreference into a full class and use that instead of our own custom items. Should we do that?

benfry added a commit that referenced this pull request Jul 25, 2014

Merge pull request #2568 from joelmoniz/presentBckColor
Added Present's background color as an option to the Preferences window.

@benfry benfry merged commit 8daa225 into processing:master Jul 25, 2014

@benfry

This comment has been minimized.

Copy link
Member

benfry commented Jul 25, 2014

Let's take the extra text field out of there and just have people click the color. It's extra complexity that's just not needed.

@joelmoniz joelmoniz deleted the joelmoniz:presentBckColor branch Jul 26, 2014

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