Forget unsaved changes to color settings

[Anastasia Baryshnikova]

USE CASE: WHAT DO YOU WANT TO DO?

Check the current color settings.

STEPS TO REPRODUCE AN ISSUE (OR TRIGGER A NEW FEATURE)

Open View > Color settings... and add one or two new colors. Don't click Apply. Click Close. Then re-open again View > Color settings...

CURRENT BEHAVIOR

The color settings reflect the unsaved changes, rather than the current settings that are visible in the matrix.

EXPECTED BEHAVIOR

The color settings should reflect whatever is currently applied in the matrix, so that the settings, in a way, act as a legend for the matrix.

DEVELOPERS ONLY SECTION

SUGGESTED CHANGE (Pseudocode optional)

none

FILES AFFECTED (where the changes will be implemented) - developers only

unknown

LEVEL OF EFFORT - developers only

minor

COMMENTS

[Anastasia Baryshnikova]

Interestingly, those settings are remembered the next time you load the same file. So, for example, you were looking at one color scheme first, then played with the colors but didn't save them, then close the file, reopen the file and the color scheme that you didn't save is now the default setting.

[Robert Leach]

Perhaps a reset button would solve this issue?

[Robert Leach]

I have created issue #438 which would essentially address this issue by providing more preset functionality. Though I think that this issue would be a reasonable step toward a more versatile implementation such as #438 lays out.

[Robert Leach]

Removing milestone: Color settings (automated comment)

[Christopher Keil]

Fixed final issues and cleaned up code

Fixed a bug where activeColorSet in ColorPicker was not set because of an unnecessary selectedItem check. Implemented ItemListener rather than ActionListener for changing the ColorSchemeType so that values aren't reset when picking the same scheme as already selected. Cleaned code. Resolves #342

→ <<cset 944ae30e139e>>

[Robert Leach]

I was thinking. This doesn't apply specifically to this issue, but given the limitations of the pull request interface, it might be a good idea to do the automatic formatting outside of the branch which is submitted as a pull request. Such a change (e.g. formatting) is a trivial level of effort, meaning that it's eligible for direct commit to master without going through a PR. So I think it could be done before branching, after PR approval, or even during branch work (if done to master and then merged into the branch - though that might be a headache). What do you think?

[Christopher Keil]

I didn't apply the formatting in this issue. My general concern is that the changes are really vast in relation to the current open issues. They might cause problems such as merge conflicts and end up masking actual code changes... I think the only way to do it painlessly is when no other branches than master exist. But that would mean to put opening new issues on hold.

[Robert Leach]

I know that my comment doesn't apply specifically to this issue. My suggestion is in fact because of the reasons you point out.

While I agree that ideally formatting should be done globally when there are no active branches, but seeing as how that never seems to coalesce and the fact that I had noted one of your PRs seemed(?) to have undergone formatting - it made me think that perhaps the strategy to use until all active branches are merged in would be to do it as we go on the files we're working on. Ostensibly, part of our procedure is to make sure no one is working on the file we're working on. If that's true, we can apply formatting and conflicts with other branches should be easy to resolve.

[Christopher Keil]

How about this procedure: between a post-review updated PR that is ready for merge and actually merging a final format has to be run every time. Only an auto-formatted PR may be merged.

[Robert Leach]

Sounds great.

[Christopher Keil]

Fixed final issues and cleaned up code

Fixed a bug where activeColorSet in ColorPicker was not set because of an unnecessary selectedItem check. Implemented ItemListener rather than ActionListener for changing the ColorSchemeType so that values aren't reset when picking the same scheme as already selected. Cleaned code. Resolves #342

→ <<cset 944ae30e139e>>