Skip to content

Conversation

@tom-anders
Copy link
Collaborator

@tom-anders tom-anders commented Feb 9, 2025

twoColumn.webm

@veloce
Copy link
Contributor

veloce commented Feb 10, 2025

@tom-anders really cool, thanks!

I'd suggest an improvement (maybe): we could try to have a full width background color for the non-inline moves (including the move index and padding).
Like what the website does actually. I think that way it would look even better.

@ijm8710
Copy link

ijm8710 commented Feb 10, 2025

Agree with Veloce! Just didn't know how to express it.

image

Did your originally analysis notation refactoring from a bit earlier prevent stray ( from sitting at end of line. And if so should that carry over here as well. It's decently minor but still slightly odd to see. Not sure if you have any thoughts to have some logic for it

@tom-anders
Copy link
Collaborator Author

@tom-anders really cool, thanks!

I'd suggest an improvement (maybe): we could try to have a full width background color for the non-inline moves (including the move index and padding). Like what the website does actually. I think that way it would look even better.

Sure, I’ll try it out. Do we want to use the new background for the old inline notation view as well then? To be consistent?

@tom-anders
Copy link
Collaborator Author

Did your originally analysis notation refactoring from a bit earlier prevent stray ( from sitting at end of line. And if so should that carry over here as well. It's decently minor but still slightly odd to see. Not sure if you have any thoughts to have some logic for it

Not 100% sure if we already have logic for that or not, but I don't think so...? Can you open a separate issue for that?

@tom-anders
Copy link
Collaborator Author

Okay, adjusted the background and also added some horizontal dividers:

Dark Theme:
Screenshot_1739224202

Light Theme:
Screenshot_1739224246

Website for comparison:

Screenshot_2025-02-10_22-51-34
Screenshot_2025-02-10_22-51-52

@veloce veloce merged commit c18e53e into lichess-org:main Feb 12, 2025
1 check passed
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.

3 participants