Add image suggestion feature to iMatrics widget

[imatricsOpenSource]

Hi! This is our first time contributing to Superdesk!

If you want to discuss any changes, we are able to hop on a call in our shared slack or discuss the changes in the comments. Happy reading!

[imatricsOpenSource]

Hi! After we pushed a commit with linting fixes, the fire:www workflow still fails, but we cannot see the linting errors anymore. It's very hard to guess what is still wrong. Is there a way around this?

[tomaskikutis]

Hi @imatricsOpenSource,

I didn't get to reviewing the code yet, but plan to do so soon. Ignore the failing fire:www task - the issue is on our side here.

[tomaskikutis]

Regarding linting errros, you should see them by clicking "details" in "CI / test (pull_request) " row. Or running npm run lint locally. Currently the failing one is ERROR: scripts/extensions/auto-tagging-widget/src/auto-tagging.tsx[53, 36]: trailing whitespace

[tomaskikutis]

Is there a specification/documentation somewhere describing how this should work? If not, maybe you could add it in the PR description?

It would also be useful to have a screen recording attached of the feature in action.

[hanstchou]

Is there a specification/documentation somewhere describing how this should work? If not, maybe you could add it in the PR description?

It would also be useful to have a screen recording attached of the feature in action.

Hi, Tomas!

Currently, there is no official documentation. Below I attach a snippet of the extended widget.
Images should be suggested based on the tags, and the editors should be able to drag and drop any images on the article. It should function similarly to how the authors are currently attaching images in the articles, but faster and with ease.

Similar to the tagging, two additional variables need to be set in the config/env such as the image URL and credentials which we will provide upon request.

[tomaskikutis]

Thanks for explaining @hanstchou. Screenshots are especially useful.

editors should be able to drag and drop any images on the article

from the code it seems it's only possible to drag the selected image and judging from the screenshot there are no visual clues for users to understand this. It would be good to either add a visual indication or make all images draggable, not only the selected one.

See my new comments on a class based component. Also finish fixing the lint issues. We have 2 linters and the second one only starts when first one doesn't report any issues. I did push a small change to make the first one pass. Main problems now are indentation style and usage of double quotes.

[hanstchou]

from the code it seems it's only possible to drag the selected image and judging from the screenshot there are no visual clues for users to understand this. It would be good to either add a visual indication or make all images draggable, not only the selected one.

See my new comments on a class based component. Also finish fixing the lint issues. We have 2 linters and the second one only starts when first one doesn't report any issues. I did push a small change to make the first one pass. Main problems now are indentation style and usage of double quotes.

Hi, Tomas!

I noticed this and we have now fixed the second lint issue. Additionally, the payload has been adjusted according to the backend changes.

The users can also drag the images on the list since they have a similar attribute (onDragStart) to the selected image. However, you have a point that there is no indication that they are draggable. However, the indicator feature has not been demanded/explained by the stakeholder.

From the UX/UI perspective, do you have an idea of how may we provide an indicator of the drag feature without needing to fill up more space in the widget for future development?

Thanks!

[hanstchou]

Hi @tomaskikutis

We think we have adjusted the necessary code according to the feedback. Thank you!

[hanstchou]

For the indicator, added a grab pointer on the images.

[tomaskikutis]

From the UX/UI perspective, do you have an idea of how may we provide an indicator of the drag feature without needing to fill up more space in the widget for future development?

One of the things I was about to suggest is using a relevant mouse cursor which you did already. Another useful thing to do would be to use a ToggleBox (not ToggleBoxNext) component from ui-framework which has a badge property. There you can pass any JSX and it will get displayed to the right of the title. There you could put an "i" icon(for information) and upon clicking it, display a popover describing the usage.

[tomaskikutis]

When you push new commits, GitHub requires me to authorize running of tests, so it might look like lint issues are fixed when they're not. You can check it locally by running npm run lint in the project root.

[hanstchou]

I've been running lint locally. It looks fine, and no error appears.

[hanstchou]

We will make another commit with your suggested approach. Thanks for feedback.

[hanstchou]

One of the things I was about to suggest is using a relevant mouse cursor which you did already. Another useful thing to do would be to use a ToggleBox (not ToggleBoxNext) component from ui-framework which has a badge property. There you can pass any JSX and it will get displayed to the right of the title. There you could put an "i" icon(for information) and upon clicking it, display a popover describing the usage.

hi @tomaskikutis
When trying to replace ToggleBoxNext with ToggleBox from superdesk-ui-framwork following issue occurs. How do I get by this, and do you have a class or icon library for importing the "i"-icon?

[tomaskikutis]

When trying to replace ToggleBoxNext with ToggleBox from superdesk-ui-framwork following issue occurs.

Ensure you add /react at the end of your import: import {IconButton} from 'superdesk-ui-framework/react';

do you have a class or icon library for importing the "i"-icon?

You can use IconButton with info-sign. Here's the list with all icons: https://ui-framework.superdesk.org/#/react/icon-font

[hanstchou]

Ensure you add /react at the end of your import: import {IconButton} from 'superdesk-ui-framework/react';

@tomaskikutis
Still a similar issue after importing from 'superdesk-ui-framework/react'. Maybe it's just a ts/lint complain. At least it seems to compile.

I pushed a commit with adjustments according to your feedback on the other points.

[hanstchou]

Hi @tomaskikutis

We have now replaced the ToggleBoxNext with a ToggleBox with a info button with popover showing the information. Hope everything is good now. Thank you!

[tomaskikutis]

@hanstchou I did push a few improvements. I'd like you to fix one more thing comment and we're good to merge it then. Take a look at my changes and see if they're not breaking anything - I think they should NOT. One more thing - ensure the server returns an empty array in IImageServerResponse['result'] if no image suggestions are available.

[tomaskikutis]

<img shouldn't be rendered at all if we don't have a url. Same for figcaption.

[tomaskikutis]

Take a look at my changes and see if they're not breaking anything - I think they should

correction: by mistake I wrote that I expect my changes to break things. I meant to write that I do NOT expect them to break anything

[hanstchou]

Hi @tomaskikutis

Thank you for all the help and feedback!
I have ensured that images contains an URL by filtering the items with no containing URL in the response.

Also, the changes you made did not break anything, however, I could not test the method superdesk.ui.article.prepareExternalImageForDroppingToEditor since my local version is not on the latest branch.

We will modify the server to ensure return of an empty list if no images were found in the server repo.

Let me know if I missed something. Appreciated!

[hanstchou]

We have now also updated the server to return empty array if no image suggestions are available.

[hanstchou]

Also, we may provide you with a key to the image tagging if you would like to try it and see the result.

[tomaskikutis]

I did check it in action on one of our instances. It's good that you've filtered images that don't have a URL, but do update usage so optional chaining is not used then. Same for caption - either don't use optional chaining if it will always be present, or don't render figcaption node when caption is not present. See my last comment

If you updated the server to never return images that don't have a src, update the typescript interface on the client to make the imageUrl mandatory and remove client-side filtering.

[hanstchou]

Hi @tomaskikutis !

Sorry that I missed the figcaption point. We have now fixed that figcaption is only rendered if there is a caption, and also removed unnecessary optional chaining in imageUrl.

On the other hand, we did only ensured that the server returns a result of an array. No filter of src in the server. Therefore we keep the filtering on the client.

Thank you for checking this through.

[tomaskikutis]

On the other hand, we did only ensured that the server returns a result of an array. No filter of src in the server. Therefore we keep the filtering on the client.

Can you filter it on the server instead?

Other than that, I don't have any other comments and we can merge it when unit tests pass.

[hanstchou]

Hi @tomaskikutis !

We have now added the filter of images with no src on the server and removed the now unnecessary filter in the client. Let's pray for the unit test to pass! Thank you for everything!

[tomaskikutis]

Thanks @hanstchou