Skip to content

Display the different lines in the variation bar.#3176

Open
HaonRekcef wants to merge 1 commit into
lichess-org:mainfrom
HaonRekcef:show-variations-notation
Open

Display the different lines in the variation bar.#3176
HaonRekcef wants to merge 1 commit into
lichess-org:mainfrom
HaonRekcef:show-variations-notation

Conversation

@HaonRekcef
Copy link
Copy Markdown
Collaborator

closes #3069

Adds a variation bar to the game notation or broadcasts, analysis board and studies.
This is especially useful for analysis or studies with a lot of lines.
Mirrors the web behavior, except that I put it under the notation, to make it easier to reach.
The mainline is highlighted and if the user simply presses next it will continue on the mainline.

I also added test cases.

Preview:

Screencast_20260514_000551.webm

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a “variations bar” UI element to improve navigation of branching move trees in analysis, studies, and broadcasts (mirroring lichess web’s variation navigation), and introduces widget tests to validate the behavior.

Changes:

  • Introduces a new VariationsBar widget that renders available continuations from the current node and allows jumping by tap.
  • Integrates the variations bar into analysis, broadcast game, and study move-tree views.
  • Adds widget tests for analysis, broadcast, and study screens to validate rendering and tap-to-jump behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/src/widgets/variations_bar.dart Adds the new variations bar widget that renders branch move buttons and handles tap-to-jump.
lib/src/view/analysis/tree_view.dart Pins the variations bar under the analysis PGN tree view.
lib/src/view/broadcast/broadcast_game_screen.dart Pins the variations bar under the broadcast PGN tree view.
lib/src/view/study/study_tree_view.dart Pins the variations bar under the study PGN tree view.
test/view/analysis/analysis_screen_test.dart Adds coverage for variations bar rendering, tapping a sideline, and mainline-next behavior.
test/view/broadcast/broadcast_game_screen_test.dart Adds coverage for variations bar rendering and tapping a variation in broadcasts.
test/view/study/study_screen_test.dart Adds coverage for variations bar rendering and tapping a variation in studies.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/src/view/broadcast/broadcast_game_screen.dart
Comment thread lib/src/widgets/variations_bar.dart Outdated
child: InkWell(
onTap: () => onJump(currentPath + child.id),
child: Container(
height: 38,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it a bit smaller intentionally, to not take too much space away from the notation (because the variationsBar can take 2 or even 3 lines) but maybe we should update it anyways? @veloce

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see it on a phone screen recording too.

I think it would be better to use the min interactive dimension of material lib indeed.

3 lines seems a lot. It should be a pretty rare case to have that many variations, right? Can we not constraint the number of lines in a way?

Could we have an horizontal scroll here if needed (for edge cases)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@veloce

Recording:

Screencast_20260519_160950.webm

Yes I think more than 5 or 6 is very rare and that will still fit in one row. And two lines of variations should fit in pretty much all screens so I think it should be fine

I think a horizontal scroll might not work well, as it is inside of the analysis Tabs that has a similar gesture.

@HaonRekcef HaonRekcef force-pushed the show-variations-notation branch 2 times, most recently from 6533197 to eb201f0 Compare May 19, 2026 14:20
@HaonRekcef HaonRekcef force-pushed the show-variations-notation branch 2 times, most recently from 69cd4cc to c134c5a Compare May 25, 2026 19:25
@veloce veloce force-pushed the show-variations-notation branch from c134c5a to 3b48a03 Compare May 28, 2026 13:37
final prefs = ref.watch(analysisPreferencesProvider);
// enable computer analysis takes effect here only if it's a lichess game
final enableServerAnalysis = !options.isLichessGameAnalysis || prefs.enableServerAnalysis;
final currentNode =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already an analysisState.currentNode, so why computing it again?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@veloce I don't think I can use them as I need the children and analysisCurrentNode doesn't have it.

 const factory AnalysisCurrentNode({
    required Position position,
    required bool hasChild,
    required bool isRoot,
    SanMove? sanMove,
    Opening? opening,
    ClientEval? eval,
    IList<PgnComment>? lichessAnalysisComments,
    IList<PgnComment>? startingComments,
    IList<PgnComment>? comments,
    IList<int>? nags,
  }) = _AnalysisCurrentNode;

final state = ref.watch(ctrlProvider).requireValue;

final broadcastPrefs = ref.watch(broadcastPreferencesProvider);
final currentNode = state.root.branchesOn(state.currentPath).lastOrNull ?? state.root;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem: there is already a currentNode in broadcast state.

displayMode: studyPrefs.inlineNotation
? PgnTreeDisplayMode.inlineNotation
: PgnTreeDisplayMode.twoColumn,
final currentNode = root.branchesOn(studyState.currentPath).lastOrNull ?? root;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem

@veloce
Copy link
Copy Markdown
Contributor

veloce commented May 28, 2026

Simulator Screenshot - iPhone 17 - 2026-05-28 at 15 46 37

In your screen recording (on android) I can see that the mainline variation is highlighted. I cannot see it on iOS though.

@HaonRekcef
Copy link
Copy Markdown
Collaborator Author

Simulator Screenshot - iPhone 17 - 2026-05-28 at 15 46 37

In your screen recording (on android) I can see that the mainline variation is highlighted. I cannot see it on iOS though.

It should be different, not sure why it is not:

                    child: Material(
                      color: isMainline
                          ? theme.colorScheme.primaryContainer
                          : theme.colorScheme.secondaryContainer,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve variation navigation during game analysis on mobile

3 participants