navigable robots table #2140

kostmo · 2024-09-06T06:31:00Z

Towards #1341. Should also be useful for #2133.

Uses the brick-tabular-list package to render the F2 robots dialog as a navigable list with tabular formatting. Hitting Tab on a selected row shows details for that robot.

Also:

Extracts some record definitions from Swarm.TUI.Model.UI into Swarm.TUI.Model.UI.Gameplay
Removes re-export of the Name type from Swarm.TUI.Model
Replace uses of maximum with the safer maximum0
New applyJust combinator

Testing

Showing a large robots list

scripts/play.sh -i data/scenarios/Challenges/Ranching/beekeeping.yaml --debug all_robots --speed 2 --autoplay

and with extra column:

scripts/play.sh -i data/scenarios/Challenges/Ranching/beekeeping.yaml --debug all_robots,robot_id --speed 2 --autoplay

Showing a small robots list with details view and logs

scripts/play.sh -i data/scenarios/Testing/562-lodestone.yaml --debug all_robots,robot_id --speed 2 --autoplay

Log view:

xsebek

This is neat. 👍

I have some nitpicks about the refactoring. It would have been nicer if that was a separate Pull Request, so that the changes here were easier to see.

The view seems to work well, but I was initially confused by the controls. It would be nice if there was a hint in UI about which keyboard shortcuts are used to control it.

src/swarm-tui/Swarm/TUI/Model.hs

src/swarm-tui/Swarm/TUI/Model/UI/Gameplay.hs

src/swarm-tui/Swarm/TUI/View/Robot.hs

src/swarm-tui/Swarm/TUI/View/Robot/Details.hs

src/swarm-tui/Swarm/TUI/View/Robot/Type.hs

src/swarm-tui/Swarm/TUI/View/Robot.hs

byorgey

Cool!

I agree the controls seem a bit confusing. To be consistent with the rest of the UI, I think Tab should only switch between focus within a single panel, not change which panel is displayed. I suggest having Enter on a highlighted robot open the robot details panel (instead of Tab), then Tab within the robot details panel just switches between the different things to focus in that panel; Esc will close it and return to the list of all robots.

byorgey · 2024-09-14T18:28:44Z

src/swarm-util/Swarm/Util.hs

@@ -273,6 +280,10 @@ applyWhen :: Bool -> (a -> a) -> a -> a
 applyWhen True f x = f x
 applyWhen False _ x = x

+applyJust :: Maybe (a -> a) -> a -> a
+applyJust Nothing x = x
+applyJust (Just f) x = f x


A couple thoughts/observations on applyJust:

applyJust = fromMaybe id

It can be generalized to compose :: Foldable t => t (a -> a) -> a -> a; compose = foldr (.) id.
I am not sure whether it is worth changing anything based on these observations but I thought they were at least worth mentioning.

Thanks, I was also intrigued by this helper function.

Is any variant of this function available in our dependencies? 🤔 The only hoogle hit is thread from some alternative prelude.

src/swarm-engine/Swarm/Game/State/Initialize.hs

xsebek

Thanks @kostmo for going through my comments. 👍

I like @byorgey's suggestion for the controls, I kept hitting Enter and closing the modal.

xsebek · 2024-09-15T19:06:29Z

src/swarm-tui/Swarm/TUI/View/Shared.hs

+tabControlFooter :: Widget Name
+tabControlFooter = hCenter $ withAttr italicAttr $ txt "NOTE: [Tab] toggles focus between panes"


If you make this polymorphic in the Name it could go to Swarm.TUI.View.Util, I think.

But maybe we will need a common set of shared view parts anyway.

xsebek · 2024-09-15T19:09:52Z

src/swarm-tui/Swarm/TUI/Model/UI.hs

@@ -8,52 +8,14 @@
 -- SPDX-License-Identifier: BSD-3-Clause
 module Swarm.TUI.Model.UI (


We don't follow this consistently (yet), but it might be nice to re-export the whole Swarm.TUI.Model.UI.Gameplay here, so that there are fewer changes in the imports.

It's a drop of water in the ocean though, so feel free to ignore this suggestion.

* use `watch` to avoid busy-waiting for the bit seed to grow Prompted by #2140, which shows hundreds of `ishere` calls. | Before | After | |--------|--------| | <img width="143" alt="Screenshot 2024-09-15 at 10 02 51 PM" src="https://github.com/user-attachments/assets/174827e4-9371-4405-a6c2-dd20233e4588"> | <img width="143" alt="Screenshot 2024-09-15 at 10 01 32 PM" src="https://github.com/user-attachments/assets/a1d02d16-487c-46d0-b5f1-5aaf4da111cb"> | This should make the testing scenario take 0.1s from previous 0.5s. You can test it with: ```sh cabal run swarm-integration -O0 -- -p '/Testing\/562-lodestone/ ```

kostmo · 2024-09-15T23:49:41Z

Tab should only switch between focus within a single panel, not change which panel is displayed. I suggest having Enter on a highlighted robot open the robot details panel (instead of Tab), then Tab within the robot details panel just switches between the different things to focus in that panel; Esc will close it and return to the list of all robots.

I have now updated the controls to behave as described. It feels a bit hacky though---perhaps at some point we would introduce the concept of a "stack" of dialogs so that things like the Esc would simply pop the top dialog from the stack, rather than special-casing logic for it.

Suggestions addressed

xsebek

Works nicely, thanks @kostmo. 👍

Continuing with the replacements started in this discussion: #2140 (comment)

kostmo changed the base branch from refactor/ui-dialogs-state-record to main September 7, 2024 06:51

kostmo force-pushed the feature/navigable-robot-dialog branch 2 times, most recently from 5435ef4 to 77a20b5 Compare September 7, 2024 07:13

restyled-io bot mentioned this pull request Sep 8, 2024

Restyle WIP navigable table #2141

Closed

kostmo force-pushed the feature/navigable-robot-dialog branch from 3a18bb6 to 76277e2 Compare September 9, 2024 06:09

kostmo changed the title ~~WIP navigable table~~ navigable robots table Sep 9, 2024

kostmo force-pushed the feature/navigable-robot-dialog branch from 76277e2 to 7cc7b13 Compare September 9, 2024 06:20

kostmo added the T-UI Involves the user interface. label Sep 9, 2024

kostmo force-pushed the feature/navigable-robot-dialog branch 6 times, most recently from deb58d0 to e3d0920 Compare September 14, 2024 01:29

kostmo requested review from xsebek and byorgey September 14, 2024 01:32

kostmo marked this pull request as ready for review September 14, 2024 01:33

navigable table

b75438e

kostmo force-pushed the feature/navigable-robot-dialog branch from e3d0920 to b75438e Compare September 14, 2024 03:41

kostmo added 2 commits September 13, 2024 22:33

cleanups

db135d8

use applyJust/applyWhen more

93fd1a5

xsebek previously requested changes Sep 14, 2024

View reviewed changes

kostmo added 8 commits September 14, 2024 13:45

preserve re-export of Name

8f8f87a

explicit export list for Swarm.TUI.View.Robot

ba1d997

add module docstring

9a300a8

rename _rob lens to _robot

19469cf

rename _rPayload lens to _row

a37214a

comment on and rename padWidth

f771ade

remove orphan Semigroup instance

1df475b

Add instructions for Tab control

a263b11

kostmo added 3 commits September 14, 2024 15:51

rename 'RobotRow' fields

e7101b5

add comment to ColumnExpansion

04cacae

explicit export list and docstring for Swarm.TUI.Model.UI.Gameplay

2c063cf

kostmo requested a review from xsebek September 15, 2024 18:04

byorgey reviewed Sep 15, 2024

View reviewed changes

xsebek reviewed Sep 15, 2024

View reviewed changes

make polymorphic in name

495f968

xsebek mentioned this pull request Sep 15, 2024

Watch for lodestone #2146

Merged

revert a usage site of applyJust

f70666b

kostmo marked this pull request as draft September 15, 2024 22:39

Fix dialog controls

cbaaf22

kostmo force-pushed the feature/navigable-robot-dialog branch from 479a682 to cbaaf22 Compare September 15, 2024 23:46

kostmo marked this pull request as ready for review September 15, 2024 23:46

kostmo requested a review from xsebek September 17, 2024 00:16

xsebek approved these changes Sep 17, 2024

View reviewed changes

kostmo added the merge me Trigger the merge process of the Pull request. label Sep 17, 2024

Merge branch 'main' into feature/navigable-robot-dialog

f29e853

mergify bot merged commit 671fd0f into main Sep 17, 2024
12 checks passed

mergify bot deleted the feature/navigable-robot-dialog branch September 17, 2024 18:28

kostmo mentioned this pull request Oct 15, 2024

use applyWhen #2180

Merged

mergify bot pushed a commit that referenced this pull request Oct 15, 2024

use applyWhen (#2180)

6ae956b

Continuing with the replacements started in this discussion: #2140 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

navigable robots table #2140

navigable robots table #2140

kostmo commented Sep 6, 2024 •

edited

Loading

xsebek left a comment

byorgey left a comment

byorgey Sep 14, 2024

xsebek Sep 15, 2024

xsebek left a comment

xsebek Sep 15, 2024

xsebek Sep 15, 2024

kostmo commented Sep 15, 2024

xsebek left a comment

		tabControlFooter :: Widget Name
		tabControlFooter = hCenter $ withAttr italicAttr $ txt "NOTE: [Tab] toggles focus between panes"

		@@ -8,52 +8,14 @@
		-- SPDX-License-Identifier: BSD-3-Clause
		module Swarm.TUI.Model.UI (

navigable robots table #2140

navigable robots table #2140

Conversation

kostmo commented Sep 6, 2024 • edited Loading

Testing

Showing a large robots list

Showing a small robots list with details view and logs

xsebek left a comment

Choose a reason for hiding this comment

byorgey left a comment

Choose a reason for hiding this comment

byorgey Sep 14, 2024

Choose a reason for hiding this comment

xsebek Sep 15, 2024

Choose a reason for hiding this comment

xsebek left a comment

Choose a reason for hiding this comment

xsebek Sep 15, 2024

Choose a reason for hiding this comment

xsebek Sep 15, 2024

Choose a reason for hiding this comment

kostmo commented Sep 15, 2024

xsebek left a comment

Choose a reason for hiding this comment

kostmo commented Sep 6, 2024 •

edited

Loading