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

Add stt smart picker #27

Merged
merged 10 commits into from
Jan 12, 2024
Merged

Add stt smart picker #27

merged 10 commits into from
Jan 12, 2024

Conversation

MB-Finski
Copy link
Contributor

This pr imports the stt smart picker functionality from the stt_helper.

Again I tried to keep the refactoring to a minimum, although, I had some hesitation about doing some refactoring for the notification flow (since you cannot use the tasks like they are used for text processing notifications).

Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
Whenever we add a migration step, it's better to bump the app version number so we make sure the step is triggered when people test the PR (i was confused for a moment).
A few minor changes and we're good to go.

src/components/AssistantPlainTextModal.vue Outdated Show resolved Hide resolved
Vue.use(VueClipboard)

export default {
name: 'AssistantPlainTextModal',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this component going to be used in other contexts than STT? If not, we could rename it to SttResultModal.

Copy link
Contributor Author

@MB-Finski MB-Finski Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must admit, I have no other definite use case for a plain text result page yet but I ventured a guess here that we might have those in the future (for example, notifications where we want to hide which text processing task type was responsible for the result) . Although, it might also be that such features would be otherwise accommodated in the notification logic?

EDIT: The copywriter functionality might be one such example where we don't want to display the result as a "free prompt" result although we're bound to use that task type as the backend.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, not a big deal. Rename it if you want, merge if you don't 😁

Signed-off-by: MB-Finski <[email protected]>
Signed-off-by: MB-Finski <[email protected]>
@julien-nc julien-nc self-requested a review January 12, 2024 10:40
@MB-Finski MB-Finski merged commit 84a6122 into main Jan 12, 2024
4 checks passed
@delete-merged-branch delete-merged-branch bot deleted the add/stt-smart-picker branch January 12, 2024 10:44
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