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

Webclient plugin fixes 11605 #1689

Merged
merged 3 commits into from
Nov 4, 2013

Conversation

will-moore
Copy link
Member

This fixes a couple of issues with webclient plugins that were uncovered during documentation writing.

To test:
Check that the centre panel is working on the main webclient page, and the right tabs are working as normal on the main page, search results page and history page (E.g Preview tab only enabled when a single image is selected, tab content loads as normal).

@atarkowska
Copy link
Member

Seems to work ok on howe. No issues

@joshmoore
Copy link
Member

@will-moore : is there any chance that this renaming will break plugins for others? /cc @chris-allan, @knabar

@chris-allan
Copy link
Member

No, this will not break existing plugins as far as the omeroweb_right_plugin() prototype is concerned. Old values of the settings associative array will still work. Still, kind of ugly. Given that this is called omeroweb_right_plugin() what exactly is the justification for the name change? Just stylistic?

I can't speak specifically to the impact of the indexing changes.

As far as whether or not the plugin is enabled/active for a particular selected element the semantics there have changed slightly in that the default is now for the a plugin to be enabled/active by default. Was making this setting optional a convenience thing?

@knabar
Copy link
Member

knabar commented Nov 4, 2013

Agreed that renaming load_tab_content should not break anything, since the old setting is still being read.

Not needing to specify plugin_index is great, is there still a way to set it manually though if I wanted to?

@will-moore
Copy link
Member Author

@chris-allan The center plugin uses "load_plugin_content" but the right plugin uses "load_tab_content" (I did this one first and was not thinking very generically). I only realised how bad this looked when I was writing the docs the other day. https://www.openmicroscopy.org/site/support/omero4-staging/developers/Web/WebclientPlugin.html The method does the same thing in both plugins, so it needs to be named the same in both.

Making plugin_enabled optional is a convenience thing, just in case someone forgot or didn't want to specify it. Previously you'd get an error and everything would break. Now it's just enabled by default.

@knabar The index is used to E.g. set the 2nd tab disabled if the 2nd right_plugin is disabled, so the index needs to match the actual index of the plugin. If you want your plugins to appear in a different order, you need to fix the RIGHT_PLUGINS and CENTER_PLUGINS settings.

@chris-allan
Copy link
Member

👍

Thanks @will-moore, all makes sense to me. Ready to merge.

joshmoore added a commit that referenced this pull request Nov 4, 2013
@joshmoore joshmoore merged commit 720493f into ome:dev_4_4 Nov 4, 2013
@will-moore will-moore deleted the webclient_plugin_fixes_11605 branch November 11, 2013 14:53
@will-moore
Copy link
Member Author

--rebased-to #1749

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants