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

Use custom editor if plugin provides one #247

Merged
merged 3 commits into from
Aug 8, 2019

Conversation

dobbs
Copy link
Member

@dobbs dobbs commented Aug 3, 2019

No description provided.

@dobbs
Copy link
Member Author

dobbs commented Aug 3, 2019

Deployed in glich along with an early draft of slackmatic plugin for experimentation.
https://great-hat.glitch.me/test-slackmatic.html

@WardCunningham
Copy link
Member

WardCunningham commented Aug 4, 2019

Thanks for offering a working example for this new work. I fumble through it.

I Choose slackmatic from the factory menu. See this.
image
I Press save. See this.
image
I Drop an image. Nothing happens.

I understand that this might be more about enabling a custom editor in factory than about this custom editor. But we should be clear what we would expect of a custom editor and that might be a way to escape from the editor, especially if it has a sequence.

lib/factory.coffee Outdated Show resolved Hide resolved
Plugin authors who offer custom editors with their plugins should
add  "customEditor": true to their factory.json

Without the pre-loading, the first time an author would choose a
plugin with a custom editor, there was a visible delay in the
interface while the plugin was loaded from the server.
lib/legacy.coffee Outdated Show resolved Hide resolved
lib/factory.coffee Outdated Show resolved Hide resolved
@dobbs
Copy link
Member Author

dobbs commented Aug 4, 2019

The updated PR and plugin are now published at glitch to see plugin get pre-loaded
https://great-hat.glitch.me/test-slackmatic.html

@WardCunningham
Copy link
Member

I may have pontificated in the wrong field to make my thoughts part of our historical record. The curious can consult #247 (comment).

@paul90
Copy link
Member

paul90 commented Aug 5, 2019

Let's pull that comment into this thread....

I like this solution. It offers a nice compromise between built-in plugins and load-on-first-use plugins, the two choices we have today. The server admin is choosing to selectively build-in a capability for all users of that farm only.

{
  "name": "Slackmatic",
  "title": "Slackmatic Plugin",
  "category": "other",
  "customEditor": true
}

The example I gave was a plugin's about page talking to itself so the choice of terms was of little consequence. Here a plugin is talking to the Factory about Factory issues where everything is custom. Maybe "custom" is redundant or "true" is missing an opportunity to be more precise. I don't have a specific suggestion. Just recognizing the uniqueness of the channel.

Maybe "custom" is not redundant, it indicates that this parameter is about a custom editor, rather than the standard built-in one. All the core client needs to know is if the plugin supplies a custom editor, so "true" (or "false") is sufficient. As long as there is a standard entry point for the custom editor, that should be quite enough in the core (together with some error handling in case the plugin doesn't really have a custom editor).

@dobbs
Copy link
Member Author

dobbs commented Aug 5, 2019

I considered naming this configuration preload in case plugins should have other reasons to be preloaded. But that seemed premature since the only use case at hand is this editor and I would have needed another configuration for the editor anyway. Ward has said a couple times that he had expected to see more innovation in editors. Naming this a customEditor does promote the existing text editor into The Official Editor where at the moment it's really The Only Editor. Perhaps dropping custom from the name is worth it.

@WardCunningham
Copy link
Member

You are convincing. I'm in favor with no complaints.

@dobbs
Copy link
Member Author

dobbs commented Aug 8, 2019

With this latest commit, I have simplified the config in factory.json as we discussed here. I've also worked a little harder than usual to provide an informative error message for future plugin authors (like my future self) that might configure the factory but mess up the plugin itself

@WardCunningham WardCunningham merged commit a21faa6 into fedwiki:master Aug 8, 2019
@paul90
Copy link
Member

paul90 commented Aug 9, 2019

Published as + wiki-client@0.18.1

@dobbs dobbs deleted the plugin-specific-editor branch November 17, 2019 07:00
@paul90 paul90 mentioned this pull request May 2, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants