Modify

Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#15410 closed defect (fixed)

Color preferences overhaul

Reported by: bastiK Owned by: team
Priority: normal Milestone: 17.10
Component: Core Version:
Keywords: Cc:

Description

Color name registration (Preferences.registerColor) and ColorPreference could do with some updates.

  • It relies heavily on fragile string matching and several things are no longer working as intended.
  • Color names are only registered at runtime for the current session, but the table shows all colors from the preference defaults. Now that JOSM remembers default preferences from the previous sessions, this leads to a mix of color names and raw preference keys in the GUI table.
  • The table model relies on a Map from translated names to preference keys (error prone).

Attachments (0)

Change History (26)

comment:1 by bastiK, 6 years ago

In 12929/josm:

see #15410 - fix GUI table displays pref keys instead of registered and translated color names

comment:2 by bastiK, 6 years ago

In 12942/josm:

see #15410 - better fix for color name display

comment:3 by bastiK, 6 years ago

In 12943/josm:

see #15410 - do not attempt to translate preference key

comment:4 by bastiK, 6 years ago

In 12944/josm:

see #15410 - better display of mappaint colors

comment:5 by bastiK, 6 years ago

In 12945/josm:

see #15410 - fix display of custom layer color

comment:6 by bastiK, 6 years ago

In 12946/josm:

see #15410 - fix deleting color entries; do not display unchanged layer colors (this gets excessive if user opens may gpx files over time)

comment:7 by bastiK, 6 years ago

In 12947/josm:

see #15410 - set cell renderer only for 2nd column, not entire table

comment:8 by bastiK, 6 years ago

In 12949/josm:

see #15410 - update test

comment:9 by bastiK, 6 years ago

In 12950/josm:

see #15410 - add custom table model

comment:10 by bastiK, 6 years ago

In 12952/josm:

see #15410 - don't map translated values to keys; locale sensitive sorting

comment:11 by bastiK, 6 years ago

In 12953/josm:

see #15410 - highlight entries with modified color; fix "set to default"

comment:12 by bastiK, 6 years ago

In 12954/josm:

see #15410 - better sorting

comment:13 by bastiK, 6 years ago

How can you acknowledge a Checkstyle warning?

comment:14 by Don-vip, 6 years ago

acknowledge? What do you mean?

comment:15 by Don-vip, 6 years ago

In 12959/josm:

see #15410 - checkstyle

in reply to:  14 comment:16 by bastiK, 6 years ago

Replying to Don-vip:

acknowledge? What do you mean?

Make the informed decision to ignore it. It's good to point out unnecessary public modifiers in interfaces, but in this case I would prefer to keep it.

comment:17 by Don-vip, 6 years ago

It's possible by adding checkstyle comments // CHECKSTYLE.OFF: xxx and // CHECKSTYLE.ON: xxx (xxx being the check name) before and after the declaration. Or simply make the class public. Why do you want to keep a public constructor on a private class?

comment:18 by bastiK, 6 years ago

It shows the intentions whether this method/constructor should be used from outside the inner class or not, even though this isn't enforced by the compiler. If you were to refactor the class and make it public, then you can simply keep the access modifier as is.

Last edited 6 years ago by bastiK (previous) (diff)

comment:19 by Don-vip, 6 years ago

OK I can see if we can tune the checkstyle test to allow it.

comment:20 by bastiK, 6 years ago

In 12982/josm:

remove old migration code (see #15410)

comment:21 by bastiK, 6 years ago

In 12983/josm:

see #15410 - move cached() convenience method to super type

comment:22 by bastiK, 6 years ago

In 12987/josm:

see #15410 - change preferences scheme for named colors - makes runtime color name registry obsolete

comment:23 by bastiK, 6 years ago

Resolution: fixed
Status: newclosed

In 12989/josm:

closes #15410 - update tests + minor fixes

comment:24 by bastiK, 6 years ago

In 12990/josm:

see #15410 - update tests + minor fixes (2)

comment:25 by Don-vip, 6 years ago

In 14152/josm:

see #15410, see #16604 - restore restart notification when mappaint colors are modified in preferences

comment:26 by simon04, 4 years ago

Ticket #2973 has been marked as a duplicate of this ticket.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.