Image lightbox module organization

Development
#1

Hello,

In the context of https://jira.xwiki.org/browse/XWIKI-19172 and https://jira.xwiki.org/browse/XWIKI-19195 I will need to organize the code inside two modules, these being the webjar and the ui (needed for the configuration section and for integrating the template of the library)

My question is where would be best to integrate them. For this there are 2 options, both under xwiki-platform-image:

  • xwiki-platform-image
    • xwiki-platform-image-ui
    • xwiki-platform-image-webjar
  • xwiki-platform-image
    • xwiki-platform-image-lightbox
      • xwiki-platform-image-lightbox-ui
      • xwiki-platform-image-lightbox-webjar

I propose to go with the first version, WDYT?

Image module organization
#2

Hi Lavinia. Your proposal is not very clear to me. I think by option 1 you mean to merge all image-related features into the same modules, right?. Right now the image modules are all about thumbnails and option 1 would mean adding lightbox to it.

Option 2, OTOH, seems to be about splitting the features and having a module for each:

* xwiki-platform-image
  * xwiki-platform-image-thumbnail
    * xwiki-platform-image-thumbnail-api
    * xwiki-platform-image-thumbnail-plugin
  * xwiki-platform-image-lightbox
    * xwiki-platform-image-lightbox-ui
    * xwiki-platform-image-lightbox-webjar

Is that so? Note that it means moving the thumbnail code to new modules.

PS for next time: always explain the WHY in your proposals so that we can understand your thinking and the reasons.

For me, if I understand correctly what you proposed, I tend to prefer option 2 because it makes it more clear what each module is about and we could imagine some flavor wanting to have one feature and not the other one.

Thanks

#4

Sorry for not being clear. Yes, what I propose by option 1 is to merge all the image-related features inside the xwiki-platform-image.
To explain the ‘why’ in my proposal, I tend to consider that since the image module is not a complex module, with only one functionality added over a long time, there isn’t a need for splitting the code more to have a good structure.

About the second option, I was proposing to leave the thumbnail code as it is, without creating a new module for it, but only for the lightbox code.

I would put the idea with xwiki-platform-image-thumbnail and xwiki-platform-image-lightbox as an option 3. This option will then include also moving the existing code to a new module.

#5

You cannot do that since it would not be symmetrical. For me there are only 2 real options. Said differently I’d vote -1 on option 2 without the relocation to a thumbnail module (also it’s not hard to do).

#6

That’s not true:

  • xwiki-platform-api provides a component to process images (ImageProcessor); it’s main operation is currently to scale images. But we can scale images both up and down. Resizing an image to make it smaller doesn’t necessarily mean we create a thumbnail: I may be resizing an image from 4000px to 2000px just to reduce the network traffic, not to create a 2000px “thumbnail”.
  • xwiki-platform-plugin provides an XWiki plugin that resizes image attachments on the server side before they are downloaded. This can be for reducing network traffic, for creating thumbnails or even for making images bigger (less likely but still possible)

So no, I don’t think the current xwiki-platform-image-* modules are only about thumbnails. I think we should keep them like that. The question for me is whether the new lightbox related code should be put directly in xwiki-platform-image-* or in a separate module (xwiki-platform-image-lightbox-*). The second option is probably the cleanest, but it also feels a bit over engineered to me (to have lots of small modules with just one wiki page or component).

So my preference goes to Option 1 (xwiki-platform-image-*), but I’m also fine with Option 2 (xwiki-platform-image-lightbox-*).

Thanks,
Marius

#7

Ok, from your 2 bullet points it seems to be about image resizing (a bit more generic than thumbnails) so I’m changing the name to xwiki-platform-image-resize then. But my point remains.

New names (aka option 3 now):

* xwiki-platform-image
  * xwiki-platform-image-resize
    * xwiki-platform-image-resize-api
    * xwiki-platform-image-resize-plugin
  * xwiki-platform-image-lightbox
    * xwiki-platform-image-lightbox-ui
    * xwiki-platform-image-lightbox-webjar

I’m -0 for option 1 (very close to -1), -1 for option 2 (without the rename of the existing code and the addition of the new submodule) and obviously +1 for option 3 (the symmetrical solution between resize and lightbox).

FTR option 1 is:

* xwiki-platform-image
  * xwiki-platform-image-api
  * xwiki-platform-image-plugin
  * xwiki-platform-image-ui
  * xwiki-platform-image-webjar

The reasons I prefer option 3 vs option 1 are:

  • I find it more clear to understand what the modules are about
  • They can be installed independently (for ex a flavor might need one and not another)
  • They don’t “pollute” each other. For example there’s no plugin for the lightbox. Or there’s no webjar that contains the resize code. There’s no api for the lightbox either.
#8

This is too much for me, really. With this reasoning we should have a separate module for each operation that can be done on an image:

  • xwiki-platform-image-resize
  • xwiki-platform-image-crop
  • xwiki-platform-image-padding
  • xwiki-platform-image-quantization
  • xwiki-platform-image-filter
    • xwiki-platform-image-filter-smoothing
    • xwiki-platform-image-filter-sharpening
    • xwiki-platform-image-filter-noise
  • and so on…

This would make sense if XWiki was an image processing library, i.e. if all these features were implement by us, but this is not the case. For all these image processing operations we would be calling third party libraries.

I don’t see xwiki-commons-xml-dom, xwiki-commons-xml-sax, xwiki-commons-xml-stax, xwiki-commons-xml-html, xwiki-commons-xml-clean or xwiki-commons-xml-parse. I only see xwiki-commons-xml.

I’d rather keep:

  • xwiki-platform-image-api for all image related Java APIs
  • xwiki-platform-image-webjar for all image related JavaScript code
  • xwiki-platform-image-ui for all image related UI

for now at least, because there’s really not that much code in them. If we see these modules grow too big sure, we’ll split them, but I don’t see this happening too soon.

So, to summarize, I’m still +1 for option 1, +0 for option 2 and -0 for option 3 (close to -1).

Thanks,
Marius

#9

BTW I don’t remember if we discussed bundling the lightbox feature in XWiki Standard or not (or if we need to). It could be a contrib extension but I feel for the best UX it would be better to be in XWiki platform and bundled in XS (with an Admin UI option to turn it off). Do we agree on this?

#10

Yes.

#11

+1.
The only cons I see to make this part of XS is if many contributors to these modules are not committers, this implies a large load of PR review.

#12

Hi,

To conclude, there is: option 1 = 2 votes, option 2 = -1 votes, option 3 = 1 vote
So I will go with option 1. But let me know if there are other remarks.

Thanks,
Lavinia

#13

Small note that you didn’t send a vote but a proposal so counting votes is not the best (they’re just non binding opinions). If you want a real vote you should resend this as a vote. This will then force the committers to reply, you’ll need to wait 72 hours and at least 3 committers need to vote (see https://dev.xwiki.org/xwiki/bin/view/Community/Committership#HVoting for details).

I think it would be better to at least have another opinion from a committer since Marius and I are not in agreement here.

#14

Regarding the modules organization. My votes are:
-1 for option 1: I feel like it is missing separation of concern between image processing (saving, resizing…) and image presentation on the front-end (lightbox), which as quite different things
+1 for option 2, either by keeping xwiki-platform-image as it is and having a xwiki-platform-image-lightbox sub-module, or by splitting xwiki-platform-image into two sub-modules xwiki-platform-image-processing, and xwiki-platform-image-lightbox
-0 for option 3: it feels to me like over-engineering, especially regarding the current size of the module code base (and we can decide later to split existing modules).

#15

There is separation. It’s not a single xwiki-platform-image module. There is:

  • -api for image processing on the server side
  • -ui and -webjar for image presentation on the client side (light box is just one type of presentation)
#16

There is no topic/business separation. Only technological separation (java API, wiki pages, webjar, plugin).

#17

This is very easy to achieve and without this you’ll have some unbalanced directory structure with no rules and a very hard time to decide where to put the next image topic.

#18

+1, I actually proposed to move the existing modules into a xwiki-platform-image-processing sub-module, and everything related to lightbox into a xwiki-platform-image-lightbox sub-module. That way xwiki-platform-image contains only two sub-modules with well defined business definitions.

#19

ok so that’s essentially option 3, just with a different name than the one I proposed (xwiki-platform-image-resize), probably to be less specific so that it can contain more stuff.

#20

The for the lightbox implementation we’ll probably need some generic code that should be reusable outside the context of lightbox presentation. For instance:

  • to collect the images found in the page (that is then feed as JSON to the lightbox library of choice)
  • to get the image attachments of the page (REST and some JavaScript)
  • showing a balloon / floating toolbar when hovering an image, where the light box can inject its button but others may do so too (so an UIX).

This if we follow your logic, 2 modules won’t be enough. And then, is xwiki-platform-image-processing enough, or do we need a child xwiki-platform-image-processing-api?

I really think we’re over-engineering this… As I said, I prefer to start simple and refactor / split as we go, when we really need it.