Skip to content
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

Temporary editor colors during editing sessions #1296

Merged
merged 11 commits into from
Jan 14, 2021
Merged

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Dec 22, 2020

First attempt towards implementing #91

For now the author annotation is only available during editing sessions

image

  • Make sure that color contrast ratio for text works properly
  • Check if we can indicate the author colors better in the avatar list
  • Since we currently use the session id as an identifier author annotation might also be lost as soon as one leaves the document and the session is cleaned up
    • should at least persist the session metadata as long as the document is open
    • otherwise we might be able to persist decorations as marks in the document which would be also useful for persisting them later on

Follow up

  • decorations are always rendered into the dom which might have a performance impact on larger files -> check if there might be a way to only render decorations if actually needed

cc @jancborchardt for design feedback :)

@juliushaertl juliushaertl added enhancement New feature or request 2. developing labels Dec 22, 2020
@juliushaertl juliushaertl added this to the Nextcloud 22 milestone Dec 22, 2020
@jancborchardt
Copy link
Member

Aaawesome! :) So if we use the same colors as for the avatars, the contrast should work, as @skjnldsv made sure that contrast is enough. :)

Even if it's only enough for WCAG A or AA level that is fine though, since it can just be turned off and full text contrast is there.


About the indication of the color in the avatar list, you are referring to the issue that when people have avatars, the placeholder color is not visible, right?

I'd say a fix here is to give the avatar a 2px border of the author color. That would work both in the list of avatars-only as well as in the menu including the names.

@jancborchardt
Copy link
Member

Alternative for the highlight color as discussed: Reduce opacity of the author color so we can keep the text black, would make it more readable. And then the highlight color looks more like a text marker pen. :)

…emirror

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl
Copy link
Member Author

Ready for review and testing

@juliushaertl juliushaertl marked this pull request as ready for review January 14, 2021 07:56
@rullzer rullzer mentioned this pull request Jan 14, 2021
14 tasks
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Code looks good as far as I can judge 👍

@rullzer rullzer mentioned this pull request Jan 14, 2021
15 tasks
@juliushaertl juliushaertl merged commit 4878545 into master Jan 14, 2021
@juliushaertl juliushaertl deleted the enh/colors branch January 14, 2021 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants