Modify

Opened 2 weeks ago

Closed 9 days ago

Last modified 9 days 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 (24)

comment:1 Changed 2 weeks ago by bastiK

In 12929/josm:

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

comment:2 Changed 2 weeks ago by bastiK

In 12942/josm:

see #15410 - better fix for color name display

comment:3 Changed 2 weeks ago by bastiK

In 12943/josm:

see #15410 - do not attempt to translate preference key

comment:4 Changed 2 weeks ago by bastiK

In 12944/josm:

see #15410 - better display of mappaint colors

comment:5 Changed 2 weeks ago by bastiK

In 12945/josm:

see #15410 - fix display of custom layer color

comment:6 Changed 2 weeks ago by bastiK

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 Changed 2 weeks ago by bastiK

In 12947/josm:

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

comment:8 Changed 2 weeks ago by bastiK

In 12949/josm:

see #15410 - update test

comment:9 Changed 2 weeks ago by bastiK

In 12950/josm:

see #15410 - add custom table model

comment:10 Changed 2 weeks ago by bastiK

In 12952/josm:

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

comment:11 Changed 2 weeks ago by bastiK

In 12953/josm:

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

comment:12 Changed 2 weeks ago by bastiK

In 12954/josm:

see #15410 - better sorting

comment:13 Changed 13 days ago by bastiK

How can you acknowledge a Checkstyle warning?

comment:14 Changed 13 days ago by Don-vip

acknowledge? What do you mean?

comment:15 Changed 13 days ago by Don-vip

In 12959/josm:

see #15410 - checkstyle

comment:16 in reply to:  14 Changed 13 days ago by bastiK

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 Changed 12 days ago by Don-vip

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 Changed 12 days ago by bastiK

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 12 days ago by bastiK (previous) (diff)

comment:19 Changed 12 days ago by Don-vip

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

comment:20 Changed 11 days ago by bastiK

In 12982/josm:

remove old migration code (see #15410)

comment:21 Changed 11 days ago by bastiK

In 12983/josm:

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

comment:22 Changed 9 days ago by bastiK

In 12987/josm:

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

comment:23 Changed 9 days ago by bastiK

Resolution: fixed
Status: newclosed

In 12989/josm:

closes #15410 - update tests + minor fixes

comment:24 Changed 9 days ago by bastiK

In 12990/josm:

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

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.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.