Opened 5 years ago

Closed 4 years ago

Last modified 19 months ago

#2405 closed enhancement (fixed)

[PATCH] Allow OList headings to be clickable and add sorting abillity

Reported by: scythetwirler Owned by: Vladislav Belov
Priority: Should Have Milestone: Alpha 19
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

OList headings should be clickable like buttons so that implementing sorting on OLists will be possible in a user-friendly manner.

Attachments (14)

COList_cpp_add_event_header_selection.patch (2.2 KB) - added by Vladislav Belov 4 years ago.
lobby_game_list_sorting_without_arrows.patch (4.3 KB) - added by Vladislav Belov 4 years ago.
Sort by columnt, if click on heading
colist_handler_lobby_sortable_game_list.patch (6.2 KB) - added by Vladislav Belov 4 years ago.
colist_handler_lobby_sortable_game_and_player_list.patch (10.3 KB) - added by elexis 4 years ago.
colist_handler_lobby_sortable_game_player_list_with_arrow.zip (7.2 KB) - added by Vladislav Belov 4 years ago.
dropup-arrow.png (313 bytes) - added by Vladislav Belov 4 years ago.
Arrow for descending sorting
Cost_js_fix_population_cost_and_bonus_v2.patch (5.0 KB) - added by Vladislav Belov 4 years ago.
colist_handler_lobby_sortable_game_and_player_list_with_arrows_v2.patch (89.1 KB) - added by Vladislav Belov 4 years ago.
colist_handler_lobby_sortable_game_and_player_list_with_arrows_v3.patch (12.9 KB) - added by Vladislav Belov 4 years ago.
arrow-down.png (292 bytes) - added by Vladislav Belov 4 years ago.
arrow-up.png (284 bytes) - added by Vladislav Belov 4 years ago.
arrows-up-down.png (351 bytes) - added by Vladislav Belov 4 years ago.
colist_handler_lobby_sortable_game_and_player_list_with_arrows_v4.patch (18.9 KB) - added by Vladislav Belov 4 years ago.
colist_handler_lobby_sortable_game_and_player_list_with_arrows_v4.1.patch (18.4 KB) - added by Vladislav Belov 4 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 5 years ago by scythetwirler

Milestone: Alpha 16Backlog

comment:2 Changed 4 years ago by Vladislav Belov

I have realized feature from #2405. But there is very interesting moment of naming: What we selected: heading, columnt or def? Now I just set "selected_def" (olist attribute) to def id, which was selected.

comment:3 Changed 4 years ago by leper

Store both what entry and what column was selected. For what was selected just use an index.

comment:4 Changed 4 years ago by Vladislav Belov

Keywords: review patch added
Summary: Allow OList headings to be clickable.[PATCH] Allow OList headings to be clickable.

Changed 4 years ago by Vladislav Belov

Changed 4 years ago by Vladislav Belov

Sort by columnt, if click on heading

Changed 4 years ago by Vladislav Belov

comment:5 Changed 4 years ago by elexis

About the last patch:

-Playerlist now sortable too -Added comment to COList.cpp -Sorts gamelist by mapsize according to actual mapsize, not to translated string. -Some simplifications.

Todo if time permits: (1) Add an arrow icon to the column header as in #3278 (2) The stuff in updatePlayerListOrderSelection and updateGameListOrderSelection could be done in c++ (so that its done only once and we dont need to repeat identical code in JS).


The patch works for both a18 and svn. You should test it in a18, since there are multiple games and players online.

The patch should be combined with #3244.

Changed 4 years ago by Vladislav Belov

Attachment: dropup-arrow.png added

Arrow for descending sorting

Changed 4 years ago by Vladislav Belov

comment:6 Changed 4 years ago by sanderd17

Could you revert the space-tab changes? It makes the patch hard to review and for XML files, the only rule is to be consistent in the file (see Coding_Conventions#XML).

The JS sorting functions can probably be done a bit nicer. You know that object.attr is the same as objattr?. So it should at least be possible to catch the name, mapSize and mapType in one case

The rest looks fine to me.

comment:7 Changed 4 years ago by Vladislav Belov

Owner: set to Vladislav Belov
Status: newassigned
Summary: [PATCH] Allow OList headings to be clickable.[PATCH] Allow OList headings to be clickable and add sorting abillity

comment:8 Changed 4 years ago by sanderd17

You should also modify the RelaxNG templates (gui.rnc and gui.rng) as the XML format has been changed.

The position of the arrows is also a bit strange IMO, they're closer to the next column header than to the header they modify. I would try to put them before on the left side of the column header, and use a circle or square when that column isn't the one used for sorting (instead of just hiding the arrow).

comment:9 in reply to:  8 Changed 4 years ago by elexis

Replying to sanderd17:

I would try to put them before on the left side of the column header

On ticket:3278#comment:11

If we go with an arrow that should be to the right of the column heading (as it is for every other gui I've encountered).

comment:10 Changed 4 years ago by sanderd17

Having a "not-sorted" icon would help with determining for which column the arrow is valid.

Next to that, most GUIs that do sorting also have column separators (different background colours, or small lines). With a column separator, the value of whitespace as a separator is less important.

Since whitespace is currently our only separator, the icon should be as close to the text as possible (unless you add a line).

Changed 4 years ago by Vladislav Belov

Attachment: arrow-down.png added

Changed 4 years ago by Vladislav Belov

Attachment: arrow-up.png added

Changed 4 years ago by Vladislav Belov

Attachment: arrows-up-down.png added

comment:11 Changed 4 years ago by sanderd17

I still get Relax NG error reports when loading the lobby.xml

Also, that dropup sprite is no longer needed I suppose?

Last edited 4 years ago by sanderd17 (previous) (diff)

comment:12 Changed 4 years ago by sanderd17

Resolution: fixed
Status: assignedclosed

In 16781:

Implement sortable columns for the gui, and use them in the lobby. Patch by Vladislav. Fixes #2405.

comment:13 Changed 4 years ago by sanderd17

Keywords: review removed
Milestone: BacklogAlpha 19

Thanks for your work. Your patch had some annoying windows line endings for the xml and js files, but I removed those manually.

Apart from that, it was a nice patch.

comment:14 Changed 3 years ago by elexis

In 17665:

Fix a bug in r16781 which prevented the replaylist from being sorted correclty on init, refs #2405, #3473.
The default column argument is given in XML and applied in COList::SetupText?() but it hasn't been stored, preventing JS from knowing the selected column.

comment:15 Changed 3 years ago by elexis

In 17668:

Don't use two different default values ("name" and "") in JS for the default lobby sorting order in r16781, refs #2405.
Supplement a variable g_GameStatusOrder missing in r17387.

comment:16 Changed 2 years ago by elexis

In 19352:

Keep RELAX NG compact syntax file describing the GUI properties in sync with the RELAX NG XML schema counterpart following r16781.
Difference reproducible with trang.

Reviewed By: Vladislav
Refs #2405

comment:17 Changed 19 months ago by elexis

In 20555:

Replace mod selection sorting choice dropdown and checkbox with the column based sorting from rP16781, refs #2405.
Use case insensitive sorting iff it's the locale default.

Note: See TracTickets for help on using tickets.