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

Add Scroll in dropdown list of a toolbar option ? #952

Open
gauravjain028 opened this issue Apr 9, 2018 · 19 comments
Open

Add Scroll in dropdown list of a toolbar option ? #952

gauravjain028 opened this issue Apr 9, 2018 · 19 comments
Labels
package:theme-lark package:ui status:discussion support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@gauravjain028
Copy link

gauravjain028 commented Apr 9, 2018

🆕 Feature request

Is it possible to show scroll bar in the dropdown in toolbar?
For example, I am using Font plugin, which has font family and size. If I have many options then it shows long dropdown. I guess it is not only related with font plugin, it is related with Dropdown view.

screenshot from 2018-04-09 12-01-12

Is there any workaround for it?

Is this a bug report or feature request? (choose one)

🐞 Bug report or documentation improvement

💻 Version of CKEditor

12.0.0

📋 Steps to reproduce

  1. Follow the steps from the Inline Widget docs (https://ckeditor.com/docs/ckeditor5/latest/framework/guides/tutorials/implementing-an-inline-widget.html) or (https://ckeditor5.github.io/misc/placeholder-demo/index.html)
  2. Try to style all the text

✅ Expected result

From my perspective, it would be desired of the inline widget to be treated as part of the text when it comes to styling(bold, italic, font...).

❎ Actual result

The text around the inline widget get styled but the widget itself does not.

📃 Other details that might be useful

I was able to get around this issue with changing the createContainerElement with createAttributeElement, adding a random id and priority 20.
And then adding the text using the private _insertChild converting elementToElement. But this does not seam right.


If you'd like to see this feature implemented, add 👍 to this post.

@Reinmar
Copy link
Member

Reinmar commented Apr 9, 2018

cc @oleq @dkonopka

Can we have some reasonable max-height?

And what would be a workaround until we change this on our side?

@Reinmar Reinmar added type:improvement This issue reports a possible enhancement of an existing feature. status:discussion labels Apr 9, 2018
@Reinmar Reinmar added this to the next milestone Apr 9, 2018
@oleq
Copy link
Member

oleq commented Apr 9, 2018

.ck.ck-dropdown__panel {
	max-height: 160px; /* or anything else, more likely ~300px or so */
	overflow-y: auto;
}

This should become a part of our codebase.

image

@Reinmar
Copy link
Member

Reinmar commented Apr 9, 2018 via email

@dkonopka
Copy link
Contributor

dkonopka commented Apr 10, 2018

Like @Reinmar I would go with something really long (at least 400px), even Google is trying to avoid "double scroll" issue with soooo long dropdowns.

I was thinking about using Viewport units, I mean - vh property, but first we need research in case of some unexpected bugs. AFAIR we didn't think about vh/vw/vmax properties before in the UI elements.

.ck.ck-dropdown__panel {
	max-height: 90vh;
	overflow-y: auto;
}

screen shot 2018-04-10 at 08 46 37

@long-lazuli
Copy link

long-lazuli commented Jul 3, 2018

That is something that could be handled by a CSS custom-property for me.

.ck.ck-dropdown__panel {
	max-height: 60vh;
	max-height: var(--ck-editor-dropdown-max-height, 60vh);
	overflow-y: auto;
}

and in the dropdown plugin, or in theme plugin, I don't know :

...
  init(){
    const editorHeight = editor.element.nextElementSibling.clientHeight
    editor.element.style.setProperty('--ck-editor-dropdown-max-height', `${editorHeight}px`)
    ...
  }
...

@dkonopka
Copy link
Contributor

dkonopka commented Jul 3, 2018

@long-lazuli CSS Variables is a really good idea to handle that and let users eaisly customize the height of dropdown.

Anyways your JS snippet might be buggy, because editor.element.nextElementSibling.clientHeight can be bigger than 100% of viewport height, so simple CSS Variable will be enough I think.

@dkonopka dkonopka self-assigned this Jul 3, 2018
@oleq
Copy link
Member

oleq commented Jul 3, 2018

@dkonopka Can you propose some PR?

@long-lazuli
Copy link

long-lazuli commented Jul 3, 2018

@dkonopka , my code is a pseudo-code to give a direction. It's not intended be pasted somewhere.
It's just to discuss some possibilities.
For the record, Math.max() should do the trick.

But it could be better to use something like

  • --ck-editor-viewport-height
  • --ck-editor-viewport-width
  • --ck-editor-toolbar-distance-from-viewport-top
  • --ck-editor-toolbar-distance-from-viewport-right
  • --ck-editor-toolbar-distance-from-viewport-bottom
  • --ck-editor-toolbar-distance-from-viewport-left

then, you can calc() things...

( I don't know if viewport height and width should be calculated in the dropdown function. maybe somewhere more generic; as we should avoid adding scroll/resize listeners in every plugins... )

oleq added a commit to ckeditor/ckeditor5-theme-lark that referenced this issue Jul 9, 2018
Fix: The `DropdownPanelView` should scroll when the content is long. Added a CSS custom property to control the height of the panel. Closes ckeditor/ckeditor5#952.
@oleq oleq modified the milestones: next, iteration 19 Jul 9, 2018
@oleq
Copy link
Member

oleq commented Jul 9, 2018

We went with the KISS approach (custom property + overflow). Let's see if it's enough first and then extend the solution when necessary (more use cases).

@dkonopka
Copy link
Contributor

Because of https://github.com/ckeditor/ckeditor5-theme-lark/issues/191 we need to revert change with maximum height and overflow of .ck-dropdown.

At this moment there is no way to fix it, firstly we need to use BalloonPanelView for tooltips.

@dkonopka dkonopka reopened this Jul 10, 2018
@oleq oleq modified the milestones: iteration 19, next Jul 11, 2018
oleq added a commit to ckeditor/ckeditor5-theme-lark that referenced this issue Jul 11, 2018
Revert: The `DropdownPanelView` should scroll when the content is long. Added a CSS custom property to control the height of the panel (see ckeditor/ckeditor5#952).
@Sora2455
Copy link

Hi. Two months later, do we have a resolution for this?

@rk-1996
Copy link

rk-1996 commented Apr 21, 2020

i have same issue in ckeditor scroll bar and i added below code in component css file and it's working great..!!

:host ::ng-deep .ck.ck-reset.ck-list {
max-height: 160px;
overflow-y: auto;
}

@oleq
Copy link
Member

oleq commented Apr 21, 2020

@rk-1996 What about #3416?

@rk-1996
Copy link

rk-1996 commented Apr 21, 2020

@oleq in this i'm not facing #3416,its not generating horizontal scroll bar.

ddd

@oleq
Copy link
Member

oleq commented Apr 21, 2020

WDYT @panr?

@panr
Copy link
Contributor

panr commented Apr 22, 2020

@oleq I only came up with this quite hacky idea...
dropdown-scroll-with-tooltip

So this can be achieved but we need to test it, because we may break some things... I'm gonna prepare a POC soon.

@panr panr self-assigned this Apr 22, 2020
@Reinmar
Copy link
Member

Reinmar commented Jul 7, 2020

It's curious whether display:flow-root could help here.

@oleq
Copy link
Member

oleq commented Jul 9, 2020

It's curious whether display:flow-root could help here.

I check this and it didn't help 😕 Because of overflow-y: auto; which is needed for the scrollbar to show up when the max-height is exceeded, the browser also enables the horizontal scrollbar (due to button tooltips being positioned) and it comes down to #3416. Again.

@oleq oleq unassigned oleq, panr and dkonopka Jul 9, 2020
@Reinmar Reinmar removed the squad:red label Jul 16, 2020
@shivakantshukla55
Copy link

shivakantshukla55 commented Apr 16, 2021

I am having the same issue with BalloonEditor I tried the above solution
i.e
.ck.ck-dropdown__panel {
max-height: 160px; /* or anything else, more likely ~300px or so */
overflow-y: auto;
}
But now my whole balloon editor is scrolling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:theme-lark package:ui status:discussion support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests