Account created on 23 April 2008, about 16 years ago
#

Merge Requests

Recent comments

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Hi @tim-diels,

I tested with 1.1.x-dev and 1.2.x-dev with the MR. They were both able to be overridden in the block configuration.

Good to be RTBC.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Thank you @apaderno, I've updated the code based on the review comments.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@apaderno

Thank you for the detailed explanation.
I'd like to get some advice about the class implement of the TextareaWithGenerativeSUmmaryWidget class. If I understand correctly, I will be able to make the implementation starting from the WidgetBase according to the hierarchy of class TextareaWithSummaryWidget

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

due to upgrade of openai-php/client to version 0.7.8, Drupal 9 may not be compatible anymore.
See details and discuss in the issue ๐Ÿ’ฌ Does rc3 still work with Drupal 9? Postponed: needs info

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Hi Andrew,

Due to the upgrade of openai-php/client:0.7.8, there are conflicts to Drupal 9.5.11 and the minimum PHP version requirements is raised to 8.1.
openai-php/client:0.7.8 requires psr/http-message ^1.1.0|^2.0.0, however Drupal 9.5.11 only use up to ~1.0.

Therefore I don't have a way to make it compatible to Drupal 9 now.

Regarding your suggestion, do you mean use two fields with the module and generate summary from one textarea field to another one?

Textarea with Summary is one combo field and easier to control the relationship. It is currently out of scope to control more than one field.

To make multiple field work with OpenAI, Augmentor AI โ†’ module could be a more flexible solution.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@Mattixonix Thanks for the information.

There is a global settings that includes the OpenAI API key and endpoint url.
What you encountered might be related to that.
However I don't see issue related to the 'edit any page content' permission, and cannot reproduce with the step.

The latest updates will catch the AJAX error, and the field settings page won't allow user to enable the feature when no API information has set.

Please kindly review with the latest update, and let me know if you find any issue.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Create a merge request for review.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Please ignore my previous comment.

Since the patch wasn't released at all, and we could recreate the block with the latest patch.
It is working fine.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@keiserjb I'm working on making a stable release for this project. I may instead push a rc3 to reflect the update.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I have created a MR for this. However this won't work unless it is merged to the project.

I hope the maintainer can accept this soon.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

amourow โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

It works for newly created block. But breaks the block that was built with 1.1.0.

TypeError: Illegal offset type in isset or empty in Drupal\views_block_override\Plugin\views\display\ViewsBlockOverride->preBlockBuild() (line 363 of modules/contrib/views_block_override/src/Plugin/views/display/ViewsBlockOverride.php).

Perhaps it needs a upgrade path.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@Mattixonix Thank you for your review.
It seems the review was based on rc1. There were many /src codes fixed in rc2 version, although it was not set as the release version in the project page.

I fixed the Readme, yaml and module files in the root folder, should be ready for review. Could you review with 1.0.x dev or I have to make another release?

Regarding the error you have, it looks weird to me, too.
Could you elaborate a little more about where you got the error, please.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@W01F, yes, I talked about this module in my talk at DrupalCon Lille.

I just have updated a newer version for a more stable output with JSON response in GPT-4 model.

Before release a stable version, it should have the security coverage.
However, it is stable to be used. Please feel free to use it and give me feedbacks.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Thank you @keiserjb, I have reviewed and committed to the repo.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@AlfTheCat, thanks for the updates, good to know it resolved the redundant batch. I finally have time to review the issues now.

I hope someone can also have a look here and let the maintainer know this is RTBC.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Great work! @lotscarpark

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Looks good. Move to RTBC

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

A single "G" to represent the logo seems good.

The logo is better to have a solid color background as it is back font color, and won't work on dark mode.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Logo looks with all the requirements of dimension and size. It is also good on both light and dark background.

(updated the img in the main description)

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I'd like to suggest two modifications to the logo.

1. The water drop could have some retouch to make the edge smooth.
2. The logo will have rounded corner, so the magnifier handle can be improved by extending to the edge of the image, or moving a little bit away from the corner, or changing the design.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

The design do provide a hint of what the module can do, may be we can enlarge the detail of the element to make the logo easier to read.
Also, I just realize the Paragraph logo is flipped horizontally.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Both proposal are good design.

I personally would prefer the second one, and we need to scale up the logo to 512px while keeping the size under 10kb.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I'm reviewing the logo on the contrib day of Lille 2023

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I suggest we remove the text from the logo to make it easy to see in the project browser.

This still need some work to further reduce the file size to 10kb.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I'm reviewing the logo on the contrib day of Lille 2023

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Updated the new proposal in the main description.

The component won't be easy to see in the Project Browser. We may need to consider how would this logo to be shown in the Project Browser.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I'm reviewing the logo on the contrib day of Lille 2023

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

The design uses the Paragraph project logo inside it and looks good to me.

However we need to scale it up to 512px square.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I'm reviewing the logo on the contrib day of Lille 2023

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

The proposal 2 looks like a better candidate to move forward.
We need to scale up the image to 512px and reduce the file size further. Change the gradient shadow should reduce physical size of the image.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

I'm reviewing the logo on the contrib day of Lille 2023

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

The scope of batch process in this module is for one translation and one node.

In your case, the batch progress shows you 1/5 is correct to me.
If you have a long text field with long text in it, the batch process may have more steps, because it will split the text into chunks and call the OpenAI API in separated steps.

TMGMT job has it's own batch process for multiple translations and nodes, I believe.
Let's focus on one translation one node here.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@AlfTheCat

The screen recording is funny.

Did you make one translation request or you did multiple at once?

Now the 1.0.x-dev can be installed via composer, would you try only include the MR patch from this issue?

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@AlfTheCat

I realized the option 2 doesn't work in the module recently, because the maintainer doesn't have the branch.
Let's make a issue for that.

There is indeed one additional field "Maximum chunk tokens" in the provider settings.
It defines how maximum tokens per chunk when sending the text in the request of OpenAI API.

Also, I have found the update here may cause two batch runner, so I made another patch to fix the issue.
Can you also try include the patch from ๐Ÿ› Reduce redundant batch runner Needs review along with this one?

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

amourow โ†’ created an issue.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

The MR needs to rebase to the latest code.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

@AlfTheCat

because the additional library to calculate the Token is added in the composer.json.

Adding the patch via the root composer.json won't install it.
Because during the composer update, the additional vendor is not yet in the composer.json of this module.

There are two ways you can do.

1. do composer require rajentrivedi/tokenizer-x in the drupal project to make sure it installed
2. Since the maintainer merge the MR, you can try require this module with version 1.x-dev

It should work for you, and please let me know how does it work.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Checking the max_width and max_height in the iframe src is 0 even when the formatter set to empty string.
Test passed. Ready for review.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Made changes to make the chunk processing and Batch API work.

๐Ÿ‡น๐Ÿ‡ผTaiwan amourow

Created a merge request to update the model to recommended gpt-3.5 and gpt-4

Production build 0.69.0 2024