Skip to content

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).

@MB-Finski MB-Finski removed the request for review from marcelklehr January 11, 2024 11:03
@julien-nc julien-nc mentioned this pull request Jan 11, 2024
@MB-Finski MB-Finski force-pushed the add/stt-smart-picker branch from 4ae9763 to 106c587 Compare January 11, 2024 12:40
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.

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
@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.

3 participants