Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

t/311: createListDropdown helper should use clickOutsideHandler #312

Closed
wants to merge 4 commits into from

Conversation

oleq
Copy link
Member

@oleq oleq commented Sep 28, 2017

Suggested merge commit message (convention)

Other: The createListDropdown helper should use clickOutsideHandler. Closes ckeditor/ckeditor5#5410.

@oleq
Copy link
Member Author

oleq commented Sep 28, 2017

@oleq
Copy link
Member Author

oleq commented Dec 11, 2017

It turned out this code still won't work.

It's because the clickOutsideHandler requires a reference to View#element in its contextElements, which is not available in the createListDropdown helper because it returns an instance which is not rendered (yet). And it makes sense, because one might want to customize the instance even further so we cannot simply render() it in createListDropdown.

Possible solutions:

  1. Implement View#render event fired after the view is rendered. Then:
    function createListDropdown( ... ) {
    	const dropdownView = createDropdown( ... );
    
    	// ...
    
    	dropdownView.on( 'render', () => {
    		clickOutsideHandler( {
    			emitter: dropdownView,
    			activator: () => dropdownView.isOpen,
    			callback: () => {
    				dropdownView.isOpen = false;
    			},
    			contextElements: [ dropdownView.element ]
    		} );
    	} );
    
    	// ...
    }
  2. Allow contextElements defined as function, so it returns the value (dropdownView.element) on demand when the actual mousedown is fired and, obviously, the dropdownView.element already exists.

@Reinmar
Copy link
Member

Reinmar commented Dec 11, 2017

The #render event sound fine, as something you'd want anyway in some other place.

@Reinmar
Copy link
Member

Reinmar commented Dec 11, 2017

Actually, sorry – #rendered because it should be fired once the element is already available (plus, there won't be a conflict with the method name).

@oleq
Copy link
Member Author

oleq commented Dec 12, 2017

I decorated View#render method instead. Seems like the easiest solution.

@oleq
Copy link
Member Author

oleq commented Dec 15, 2017

Due to recent changes in the drop-down API, this PR became obsolete. Replaced by #353.

@oleq oleq closed this Dec 15, 2017
@oleq oleq deleted the t/311 branch December 15, 2017 09:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

createListDropdown helper should use clickOutsideHandler
2 participants