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

Made showPanel() and hidePanel() protected #105

Merged
merged 9 commits into from
Apr 14, 2017
Merged

Made showPanel() and hidePanel() protected #105

merged 9 commits into from
Apr 14, 2017

Conversation

oskarwrobel
Copy link
Contributor

@oskarwrobel oskarwrobel commented Apr 13, 2017

Suggested merge commit message (convention)

Feature: Made Link Plugins #showPanel() and #hidePanel() methods protected. Closes ckeditor/ckeditor5#4779. Closes ckeditor/ckeditor5#4776.


It makes possible to fully test these methods and then test how link uses them without a need to execute them.

@oleq oleq changed the title Made showPanel() and hidePanel() public Made showPanel() and hidePanel() protected Apr 14, 2017
@oleq oleq merged commit 5a83b70 into master Apr 14, 2017
@oleq oleq deleted the t/95a branch April 14, 2017 12:33
@Reinmar
Copy link
Member

Reinmar commented Apr 15, 2017

Making something protected instead of private can't close tickets which are not precisely about making stuff protected. This is really confusing. I can see the explanation that they are now testable... but there's no "Feature" here (it's an internal change if it doesn't concern public stuff) and the description of the PR only mentions changing the visibility and nothing about #94 or #95. There must be a direct link between one and another.

You need to think as an external developer when writing these things. This might've made some sense for you but even for me, this is really confusing.

EDIT: I can see that @oleq improved the message when merging this PR. Good catch. But please, do remember to format code names (#hidePanel()). Besides, the functions are actually called _hidePanel() and _showPanel() ;)

PS, please check https://github.com/ckeditor/ckeditor5-link/issues/45.

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.

Broken focus management in editor-inline LinkFormView#init() is fired after it is destroyed
3 participants