Repository navigation
Editor: Show own presence in collaborative editing sessions #76413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
72f480f
Editor: Align user block selection highlight with collaborator highlight
dabowman 615e188
Editor: Add current user to collaborators-presence list
dabowman 520f284
Show own presence (as a preference)
chriszarate c0a0fdc
Editor: Use theme color for own cursor in collaborative sessions
dabowman dfd9783
Editor: Suppress blinking cursor line for own cursor in collaborative…
dabowman 6fdb9cb
Editor: Always show own avatar in toolbar and collaborators list
dabowman e2c567d
Editor: Use theme color for own block highlight in collaborative sess…
dabowman e2415c2
Editor: Remove own block highlight from collaborative sessions
dabowman 0541d9a
Editor: Fix avatar contrast ring using pseudo-element instead of inse…
dabowman 01e847f
Update editor CHANGELOG for avatar contrast ring fix
dabowman c98b78b
Merge trunk into feat/rtc-self-presence
dabowman 6438722
Editor: Suppress selection rects for own cursor in collaborative sess…
dabowman ac17428
Editor: Fix self-presence e2e test selectors to match component imple…
dabowman 0ffef11
Editor: Update self-presence preference copy and fix e2e tests
dabowman 100a8be
Editor: Show "You" label on own cursor badge in collaborative overlay
dabowman c244a9e
Editor: Remove unused list-item-you CSS rule from collaborators list
dabowman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dabowman this change is quite aggressive for the default editing experience, especially in some darker contexts. Can you expand on the context for this change? I understand that it can be a constrast thing, but I would love for us to find something slightly more balanced/considered, as the hover-border is now about 3px around the whole block:
If we need a halo, can it be 1px at most, on the inside or outside but not both? And can it be semi opaque?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's definitely bright on dark backgrounds.
We were getting feedback to make the users own presence clearer. The approach we decided on was to include the user in the list of collaborators and make their outline style match the collaborators' to reinforce the pattern and make presence indication consistent.
I think that's a good approach but ... contrast is an issue. I dug into this quite a bit earlier on and there are some potential solutions to make the style and other styles in the UI more responsive to their context so we're not using this belt and braces approach to contrast when we don't need to. But all of them were either super complex or kinda unreliable so I ended up going with the approach you see to make sure we covered our bases. But the contrast can be too much.
On collaborators, the contest is more needed for a11y. But I think that's less of a concern for the highlight style we're talking about here since the user has the block toolbar etc. to show their focus. The issue is just consistency and how we want to align it with the collaborators without introducing other issues.
I'm totally fine just rolling it back if you think that's the right call. I think we'll want to make it all consistent eventually but we might be able to cook up a more clever way of doing that across the UI.
I think we can make the case for doing it either way. Increasing the transparency on the outline might be the way to go. Or just ditching it. Whatever looks good to your eye. I was deep in the a11y requirements and the odd constraints of CSS and the editor canvas' odd way of applying styles when I made this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tempted to do without this as it is part of a piece of mixin code whose consequences are a bit hard to predict. But the motivation is nevertheless very good, so I took a stab at a refinement here: #76619
Let me know what you think!