t/123: Implemented smart DropdownView panel positioning
#449
Conversation
| * @default 'se' | ||
| * @member {'se'|'sw'|'ne'|'nw'} #position | ||
| */ | ||
| this.set( 'position', 'se' ); |
There was a problem hiding this comment.
I like this option but I'm wondering about one case. I want to display panel using custom position but I if this position will be out of the viewport then I want to see the panel in some other optimal position that fit the viewport. So, I'm wondering about custom + auto. Does it make sense?
There was a problem hiding this comment.
Feels like a complex case to me. Is there really any use case for that ATM?
Anyway, if we moved this call https://github.com/ckeditor/ckeditor5-ui/pull/449/files#diff-27dea69116225060a1623b7476d8960fR258 to a separate method, which someone could override, all they would need to do is to pass the positions to the array in their preferred order. getOptimalPosition returns the first position which is fully visible, so order matters.
I can change the implementation to make this possible but I don't feel like we really need that ATM and just for the sake of simplicity, I'd stick with this code.
There was a problem hiding this comment.
Anyway, since the current implementation is event-driven, they could attach the same event which callback is executed later and they can run the getOptimalPosition logic once again or return whatever they want. Hackish but possible.
| createPositionedDropdown( 'NW' ); | ||
| createPositionedDropdown( 'NE' ); | ||
| createPositionedDropdown( 'SW' ); | ||
| createPositionedDropdown( 'SE' ); |
There was a problem hiding this comment.
It would be nice to see in this manual test dropdown with auto settings too I think.
Suggested merge commit message (convention)
Feature: Implemented configurable, smart
DropdownViewpanel positioning. Closes ckeditor/ckeditor5#5286.Also fixed some dropdown–related documentation and manual tests.
Additional information
I fixed some outdated docs and manual tests in this branch, which were abandoned as the API changed in the past. The UI framework guide also needed revisiting and changes are in a separate PR in the root repository, so it's nice if they were verified and merged too.