Skip to content

feature/revision italicized moves left out for broadcast mainlines#3170

Open
MaartenD wants to merge 2 commits into
lichess-org:mainfrom
MaartenD:feature/revision-italicized-moves-left-out-for-broadcast-mainlines
Open

feature/revision italicized moves left out for broadcast mainlines#3170
MaartenD wants to merge 2 commits into
lichess-org:mainfrom
MaartenD:feature/revision-italicized-moves-left-out-for-broadcast-mainlines

Conversation

@MaartenD
Copy link
Copy Markdown
Contributor

Made moves added to a broadcasts game italic.

Also provided tests.

Moves-entered-after-broadcasts-game-in-italic.mp4

Fixes: #3162

final state = container.read(ctrlProvider).requireValue;
final lastBranch = state.root.branchesOn(state.currentPath).lastOrNull;

expect(lastBranch?.isUserAdded, isTrue);
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 don't like this way of testing things. Instead of testing a value of the state we should always try to check the UI (that is the reason for widget testing).

So instead, find the corresponding move text in the UI and check that it has the italic style.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed the test. If it's not what you want, please let me know. Still learning.

final lastBranch = state.root.branchesOn(state.currentPath).lastOrNull;

// promoteAt only reorders children — isUserAdded stays false after promotion.
expect(lastBranch?.isUserAdded, isFalse);
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.

check UI, not state

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed the test. If it's not what you want, please let me know. Still learning.

final state = container.read(ctrlProvider).requireValue;
final lastBranch = state.root.branchesOn(state.currentPath).lastOrNull;

expect(lastBranch?.isUserAdded, isFalse);
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.

Check UI, not state

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed the test. If it's not what you want, please let me know. Still learning.

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.

Feature revision: italicized moves left out for broadcast mainlines

2 participants