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

The live preview of the table cell background should work with multiple-cell selection #6446

Closed
oleq opened this issue Mar 16, 2020 · 5 comments · Fixed by ckeditor/ckeditor5-theme-lark#282
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:table type:bug This issue reports a buggy (incorrect) behavior.

Comments

@oleq
Copy link
Member

oleq commented Mar 16, 2020

📝 Provide detailed reproduction steps (if any)

When the selection is in a cell, it works just fine. But when we display selected cell styles for multi-selection, they override the background.

This is almost impossible to fix unless we drop selected table cells' backgrounds completely in favor of borders.

2020-03-16 13 34 41

📃 Other details

Somehow but not directly related #6425.

cc @panr


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@oleq oleq added type:bug This issue reports a buggy (incorrect) behavior. package:table domain:ui/ux This issue reports a problem related to UI or UX. labels Mar 16, 2020
@oleq oleq added this to the iteration 30 milestone Mar 16, 2020
@panr
Copy link
Contributor

panr commented Mar 16, 2020

Yeah, I know. Just wanted to issue it too 🙈

This is almost impossible to fix

Not really, when we move selection color to :after element then we can create a sami-transparent overlay over the cells. This may cause some other issues (related to position: relative on td.ck-editor__editable_selected), but... we can try it ;-)

@mlewand mlewand modified the milestones: iteration 30, nice-to-have Mar 16, 2020
@Reinmar Reinmar modified the milestones: nice-to-have, iteration 31 Mar 24, 2020
@Reinmar
Copy link
Member

Reinmar commented Mar 24, 2020

@panr Could you make a quick POC?

@oleq
Copy link
Member Author

oleq commented Mar 24, 2020

I checked and

td.ck-editor__editable.ck-editor__nested-editable {
    position: relative;
}
td.ck-editor__editable.ck-editor__nested-editable::after {
    content: "";
    background: red;
    position: absolute;
    top: 0;
    left: 0;
    right: 0;
    bottom: 0;
    pointer-events: none;
    opacity: .2;
}

gives promising results

image

@oleq
Copy link
Member Author

oleq commented Mar 24, 2020

Related #6425.

@panr
Copy link
Contributor

panr commented Apr 16, 2020

Having this fix ckeditor/ckeditor5-theme-lark#282 I think we can also improve :focus state and deal with this #6425, which I rethinked due to the some obstacles in styling multi selection to look more like a native solution. We can leave our blueish layer over the cells then.

oleq added a commit to ckeditor/ckeditor5-theme-lark that referenced this issue Apr 17, 2020
Other: Introduced the table cell overlay to improve the rendering of multiple–cell selections and bring compatibility with styled table cells. Closes ckeditor/ckeditor5#6446.
mlewand pushed a commit that referenced this issue May 1, 2020
Other: Introduced the table cell overlay to improve the rendering of multiple–cell selections and bring compatibility with styled table cells. Closes #6446.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:table type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants