Modify

Opened 11 months ago

Closed 11 months ago

Last modified 33 hours ago

#21319 closed enhancement (fixed)

[PATCH] Refactoring of class hierarchy around JosmComboBox / AutoCompComboBox

Reported by: marcello@… Owned by: team
Priority: major Milestone: 21.10
Component: Core Version:
Keywords: Cc:

Description

Added classes in bold to orthogonalize hierarchy:

JComboBox JComboBoxModel JComboBoxEditor JTextField
JosmComboBox JosmComboBoxModel JosmComboBoxEditor JosmTextField
AutoCompComboBox AutoCompComboBoxModel AutoCompComboBoxEditor AutoCompTextField

Code reuse: JosmComboBox now uses a JosmTextField as editor
Code reuse: AutoCompComboBox now uses AutoCompTextField as editor
JosmComboBox uses more of the original L&F
JosmComboBox lists now expand all the way to the bottom or the top of the screen
Variable height items in combobox lists now work #19321
Autocomplete uses different algorithm #21290
editable="false" comboboxes in Presets now work #6157 #11024 #18714
The user may toggle LTR-RTL script in JosmTextField (menu and ctrl+space)
LTR-RTL automatically toggles according to key in AddTag and EditTag dialogs #16163

Attachments (3)

21319.patch (181.8 KB) - added by marcello@… 11 months ago.
josm_preset_missing_optional_text.jpg (35.5 KB) - added by skyper 11 months ago.
missing text
21319-2.patch (867 bytes) - added by marcello@… 11 months ago.

Download all attachments as: .zip

Change History (32)

Changed 11 months ago by marcello@…

Attachment: 21319.patch added

comment:1 Changed 11 months ago by Don-vip

Wow, that's quite a patch

comment:2 Changed 11 months ago by Don-vip

Does it mean your patch solves #21290, #6157 and #16163 all at once?

comment:3 Changed 11 months ago by Don-vip

Milestone: 21.09

comment:4 Changed 11 months ago by marcello@…

It fixes #6157, proposes a solution for #16163, and hopefully fixes #21290 (lacking a Mac I cannot test it).

comment:5 Changed 11 months ago by Don-vip

excellent!

comment:6 Changed 11 months ago by Don-vip

Priority: normalmajor

comment:7 Changed 11 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 18221/josm:

fix #21319 - Refactoring of class hierarchy around JosmComboBox / AutoCompComboBox (patch by marcello):

  • Code reuse: JosmComboBox now uses a JosmTextField as editor
  • Code reuse: AutoCompComboBox now uses AutoCompTextField as editor
  • JosmComboBox uses more of the original L&F
  • JosmComboBox lists now expand all the way to the bottom or the top of the screen
  • Variable height items in combobox lists now work, see #19321
  • Autocomplete uses different algorithm, fix #21290
  • editable="false" comboboxes in Presets now work, fix #6157 see #11024 see #18714
  • The user may toggle LTR-RTL script in JosmTextField (menu and ctrl+space)
  • LTR-RTL automatically toggles according to key in AddTag and EditTag dialogs, fix #16163

comment:8 Changed 11 months ago by Don-vip

In 18222/josm:

see #21319 - fix NPE

Changed 11 months ago by skyper

missing text

comment:9 Changed 11 months ago by skyper

I have some missing <optional> with or without additional text which only appear when I open the preset for the second time. The first time opening the preset after restart/reload empty space is present but the text is missing. Is this related to this ticket?

missing working
missing textwater_well preset screenshot

comment:10 Changed 11 months ago by skyper

Resolution: fixed
Status: closedreopened

Just tried with r18218 which works as expected, so I blame r18221.

Changed 11 months ago by marcello@…

Attachment: 21319-2.patch added

comment:11 Changed 11 months ago by marcello@…

Oops. Sorry. Patch.

comment:12 Changed 11 months ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 18223/josm:

fix #21319 - fix optional regression (patch by marcello)

comment:13 Changed 11 months ago by skyper

Does this patch also fix <optional text="Hallo world">?

Last edited 11 months ago by skyper (previous) (diff)

comment:14 Changed 11 months ago by marcello@…

where do you see that?

comment:15 Changed 11 months ago by skyper

<optional> has the option to add text instead of the default with text="".

In my screenshots above it is <optional text="Only in case of manual actuator or manual mechanical drive: "> and one of many examples in current defaultpresets.xml is <optional text="Edit Highway Attributes:"> in "Cycle Lane/Track" preset.

comment:16 Changed 11 months ago by skyper

Just tried r18224 and it works. Thanks for the quick fix.

comment:17 Changed 11 months ago by gaben

Hmm, combo boxes don't seem to work when a value has already entered. You can check it eg. in the restaurant preset.

comment:18 Changed 11 months ago by gaben

Ah sorry, multiselect is broken, not combobox. r18218 works, r18222 not.

comment:19 in reply to:  18 Changed 11 months ago by skyper

Replying to gaben:

Ah sorry, multiselect is broken, not combobox. r18218 works, r18222 not.

Cannot reproduce with r18237 and flatlaf (Dracula).

comment:20 Changed 11 months ago by gaben

For me, both Win and Linux affected with default LaF, r18237. @skyper, can you please test with native laf?

comment:21 Changed 11 months ago by gaben

Oh okay, so the multiselect is only "broken" when the tag has only a single value. Also if the value contains extra space between the semicolon separators eg. cuisine=asian; african.

By broken I mean greyed out, so I can not use the GUI for adding items.

comment:22 Changed 11 months ago by skyper

Ok, I can reproduce it if an unknown value is present (including leading or trailing white spaces). This is nothing new and happens with r18193, too. The problem is that, atm, unlike combo no values are add to the list and cause of that not all values are displayed. To stop users from overwriting valid but unknown values the multiselect is disabled. Please, open a new enhancement ticket, if you think this should be changed. Thanks.

comment:23 Changed 11 months ago by gaben

Created a new ticket, see #21385.

comment:24 Changed 11 months ago by skyper

Another possible regression: #21397

comment:25 Changed 10 months ago by Don-vip

Milestone: 21.0921.10

Milestone renamed

comment:26 Changed 10 months ago by GerdP

Another possible regression: #21438

comment:27 Changed 9 months ago by GerdP

another regression? #21619

comment:28 Changed 3 months ago by taylor.smock

Another regression: #22055

comment:29 Changed 33 hours ago by taylor.smock

In 18538/josm:

JosmTextField: setHint now properly returns the old hint

See #21319/r18221. This additionally adds a non-regression test.

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.