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.

Sign up for GitHub

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

Jump to bottom

Replace overflow:hidden/auto with display:flow-root #6795

Closed
Reinmar opened this issue May 12, 2020 · 8 comments
Closed

Replace overflow:hidden/auto with display:flow-root #6795

Reinmar opened this issue May 12, 2020 · 8 comments
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:theme-lark package:ui resolution:expired This issue was closed due to lack of feedback. status:stale type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented May 12, 2020 •
edited
Loading

📝 Provide a description of the improvement

We're using overflow:hidden or overflow:auto in order to extend a container to all its floated content.

However, it's now possible to use display:flow-root which has the same effect but does not affect visibility.

One of the places that we should re-style is the editable that currently uses overflow:auto which makes displaying some floating things or e.g. tooltips in the editable troublesome.

If you'd like to see this improvement implemented, add a 👍 reaction to this post.
@Reinmar Reinmar added type:improvement This issue reports a possible enhancement of an existing feature. domain:ui/ux This issue reports a problem related to UI or UX. squad:red package:theme-lark package:ui labels May 12, 2020
@Reinmar
Copy link
Member Author

Reinmar commented May 12, 2020

cc @oleq WDYT?

@oleq
Copy link
Member

oleq commented May 12, 2020

Certainly requires some research but makes sense.

@oleq oleq mentioned this issue May 12, 2020
Merged
@Reinmar
Copy link
Member Author

Reinmar commented May 12, 2020

Certainly requires some research but makes sense.

What Oleq meant is that we need to test that it works fine :D

Reinmar added a commit that referenced this issue May 14, 2020
@Reinmar
Merge pull request #6794 from ckeditor/i/407-simple
dbf24a2 
Feature (widget): Brought the feature allowing users to type in tight spots around block widgets where web browsers do not allow the caret to be placed (see #407). Closes #6740. Closes #6688. Closes #6689. Closes #6695.

Internal (horizontal-line): Updated the styling of .ck-horizontal-line from overflow: hidden to display: flow-root to allow the typing around UI inside the widgets (see #407, #6795).

Tests (image): Updated various image test to take the widget typing around feature into account (see #407).

Fix (media-mebed): The media widget conversion should not discard widget internals (drag or resize handlers, buttons to insert paragraphs, etc.) injected by other features when converting the URL (see #407).

Feature (theme-lark): Brought styles for the feature allowing users to type in tight spots around block widgets (see #407).

MINOR BREAKING CHANGE: The new --ck-color-focus-border-coordinates CSS custom property has been added and the existing --ck-color-focus-border property now uses it internally. If your integration overrides the latter, we recommend you update the former to avoid compatibility issues with various editor UI features.
@Reinmar Reinmar added this to the nice-to-have milestone May 18, 2020
@Reinmar Reinmar modified the milestones: nice-to-have, iteration 33, iteration 34 Jun 16, 2020
@oleq
Copy link
Member

oleq commented Jul 9, 2020

I checked our codebase and there are just a few places we can change this:

  • Replacing overflow: auto with display: flow-root on .ck.ck-editor__editable_inline works OK but it's a breaking change for integrators who used height: Npx to fix the editor height (scrollable content). They will need to add overflow: auto (or scroll) to their integrations after upgrading.
  • display: flow-root works fine for .ck-content blockquote.

In other cases overflow is mostly used together with text-overflow: ellipsis or to fix problems like this one #4647.

WDYT @Reinmar? Are we OK with this breaking change?

@Reinmar Reinmar unassigned oleq Jul 9, 2020
@Reinmar Reinmar removed the squad:red label Jul 9, 2020
@Reinmar Reinmar modified the milestones: iteration 34, nice-to-have Jul 9, 2020
@Reinmar
Copy link
Member Author

Reinmar commented Jul 9, 2020

I wouldn't do anything for now. This breaking change would not be justified here. Let's see if we'll be asked about the tooltips or similar issues often enough to change our mind.

@scofalik
Copy link
Contributor

Blockquote + horizonal rule = widget handles are not visible:

@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 12, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:theme-lark package:ui resolution:expired This issue was closed due to lack of feedback. status:stale type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Reinmar @oleq @scofalik @pomek @CKEditorBot