search

spacecowboy / Feeder

Android RSS reader app
GNU General Public License v3.0
1.63k stars 99 forks source link

added article summary with OpenAI integration #399

Closed anod closed 5 days ago

anod commented 4 weeks ago

Add summarize feature which fetching an article content and summarizes it using OpenAI API. Api key needs to be provided in the settings

image image image

anod commented 4 weeks ago

@spacecowboy please take a look

spacecowboy commented 4 weeks ago

Sure. Let me know when you think the functionality is done.

The structure seems reasonable. I noted some hard-coded english strings and some lack of padding in layouts, but I'll hold off on reviewing in detail until later.

What issues are you having with "theming of buttons and progress bars"?

anod commented 3 weeks ago

Sure. Let me know when you think the functionality is done.

The structure seems reasonable. I noted some hard-coded english strings and some lack of padding in layouts, but I'll hold off on reviewing in detail until later.

What issues are you having with "theming of buttons and progress bars"?

theme is not being applied

image

image

spacecowboy commented 3 weeks ago

Theme isn't applying because you are importing the wrong assets.

import androidx.compose.material.LinearProgressIndicator

Only import material3 assets.

Exception to that is things from the icons library. But everything else should only be material3

anod commented 2 weeks ago

Thanks forgot about material 1

spacecowboy commented 2 weeks ago

Converted the PR to a draft. Please let me know when it's ready for review

anod commented 2 weeks ago

Converted the PR to a draft. Please let me know when it's ready for review

What's missing? I don't plan to add anything

spacecowboy commented 2 weeks ago

Converted the PR to a draft. Please let me know when it's ready for review

What's missing? I don't plan to add anything

Well CI is failing for one.

anod commented 2 weeks ago

@spacecowboy Since I can't run CI, please run and let me know if it passes

spacecowboy commented 2 weeks ago

@spacecowboy Since I can't run CI, please run and let me know if it passes

Flipped a switch so it should run automatically from now on

spacecowboy commented 2 weeks ago

Also, please stop force-pushing. It makes it hard to understand what changes.

I will squash merge your PR when/if it's merged anyway

anod commented 2 weeks ago

Also, please stop force-pushing. It makes it hard to understand what changes.

I will squash merge your PR when/if it's merged anyway

I use rebase, I prefer my changes to be a single commit on top any other change and not spread across timeline, you can see and review all the changes in "Files changes" tab

anod commented 1 week ago

I think it can be a cool feature to the app. Thanks for working on it!

I have noted some issues inline, and there is a problem with the settings layout on tablets:

tablet_margins

But the biggest thing is the settings UX. It has a save/cancel button in a UI where everything else saves directly on edits.

As I see it, it should either:

* be changed so it's only the Editable section that's used. And save the fields directly as they are edited. But that may be non-ideal.

* or change so that the `OpenAI Integration` section only has one settings, a text button which opens a new screen/dialog, in which a `save` and `cancel` buttons can be present. Kind of how the `Edit feed` screen works. The `Block list` setting works like this and opens a popup. While `Device sync` opens an entire screen.

What to you think?

Will change to dialog, I just don't like dialogs as a UX pattern and with Compose it much easier to have inline experience, but I agree that it might be better for familiarity

Also I have more ideas for integration:

  1. Summarize in specific language
  2. Add follow-up options: give more details, or shorten
  3. Integrate with catalog to summarize titles
spacecowboy commented 5 days ago

Sorry for the delay in review. I think this looks good now. Thanks for your hard work :+1: