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

Feature/osgi support #13

Merged
merged 2 commits into from
May 14, 2018
Merged

Conversation

steinarb
Copy link

Adds OSGi support to TouchKit 5.0.1-SNAPSHOT, the changes are:

  1. Make the jar file be an OSGi bundle, with resolution:=optional on all imports
  2. Add an apache karaf feature repository defining a feature called "touchkit" and attach it to the touchkit maven artifact (can be installed in karaf with the command "feature:install touchkit")
  3. Add an OSGi component publishing the TouchKit widgetset (as outlined in Vaadin OSGi Support)
  4. Add an OSGi component publishing the "touchkit" theme (as outlined in Vaadin OSGi Support)

@steinarb
Copy link
Author

Right...! Code using OSGi support won't build with vaadin version <8.1.0 and so will fail with the vaadin 8.0.0 of TouchKit 8.4.1. I will have to make the pull request based on the vaadin version upgrade

@steinarb
Copy link
Author

On the other hand: if the fix to #9 is done first, then the travis-ci build will run fine when this change is pulled in, and this pull request will merge without conflicts with the pull fixing #9.

So I'll just let this pull request be.

@mstahv
Copy link

mstahv commented May 14, 2018

Awesome, looks good to me!

BTW. Could vaadin-osgi-integration dependency be optional or provided?

@mstahv mstahv merged commit aa9b7e4 into parttio:master May 14, 2018
@mstahv
Copy link

mstahv commented May 14, 2018

I merged this right away to see the build result. We can see what to do with the osgi-integration dependency still.

@steinarb
Copy link
Author

Great that you've merged it in! Thanks! :-)

The OSGi support should not interfere with with touchkit usage as a regular jar, so I don't think there is any need to make it optional.

The support consists of:

  1. Custom MANIFEST.MF headers that are ignored by non-OSGi application
  2. Two small Java classes that register the resources with Vaadin OSGi support and are not seen or used by non-OSGi systems (and instantiating the classes and calling the single method will be a harmless no-op outside of OSGi)
  3. An apache karaf feature repository file that is harmless to non-karaf systems, it's just an XML file attached to the touchkit jar maven artifact

@steinarb
Copy link
Author

Oops! The pull request also contained this one: 364cfe8

I pushed it to the OSGi branch after making the pull request and wasn't aware that the pull request would contain anything I pushed to the branch.

What the commit does is to embed a font from the "base" theme used by the TouchKit widgetset. Without this change the arrows are lacking from NativeSelect dropdown and in NavigationView navigation links, when running in OSGi.

But I don't know what the effect would be for two different jars containing the same resource in a non-OSGi setting?

I'm also not sure if there are copyright considerations by embedding the font in touchkit?

@steinarb
Copy link
Author

steinarb commented May 22, 2018

False alarm: the embedding of the font from base, is in the pull request now, but wasn't at the time the pull request was merged, and therefore isn't on master.

This part of github pull request behaviour was a bit confusing.

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.

2 participants