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

[Feature] Highlight to speech #147

Merged
merged 7 commits into from
Dec 30, 2023
Merged

Conversation

ProgramComputer
Copy link

Added an option in settings to pronounce word on mouseover.

@HugoFara HugoFara added enhancement Develop an existing feature ux User Experience could be better labels Dec 25, 2023
INSTALL.sh Show resolved Hide resolved
@HugoFara HugoFara added the invalid This doesn't seem right label Dec 25, 2023
@HugoFara
Copy link
Owner

Text-to-speech

I'm having giving a try to your work, I think this feature can be quite cool for some users, which is nice. However, it can become a bit violent when it keeps uttering words so I'm thinking of a third way.

Basically, we would keep your work, but adding another option: "click to speech". When a user clicks a word dispatches speech. The options would look like that:

New speech dispatcher

What do you think of this?

Changes to INSTALL.sh

This is a different issue that should not come in this pull request, so I won't accept them here. I'm not sure why you want these changes either, but I'm open to discussion!

@ProgramComputer
Copy link
Author

ProgramComputer commented Dec 26, 2023

Text-to-speech

I'm having giving a try to your work, I think this feature can be quite cool for some users, which is nice. However, it can become a bit violent when it keeps uttering words so I'm thinking of a third way.

Basically, we would keep your work, but adding another option: "click to speech". When a user clicks a word dispatches speech. The options would look like that:

New speech dispatcher

What do you think of this?

Changes to INSTALL.sh

This is a different issue that should not come in this pull request, so I won't accept them here. I'm not sure why you want these changes either, but I'm open to discussion!

I've changed back to localhost. Your UI modification as a dropdown is a nice addition. LWT does modify server variables, check for "set globals" such as "max_heap_table_size" and "tmp_table_size". These are not being modified at runtime and only after session restart do these changes appear. Not a concern if SQL interactions are handled safely. This line sudo mysql -e "GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO $user@$host" prevents the need to restart the session after a text is parsed. If vulnerability is still a risk, the line can be removed. It resolved my error below when I installed demo db.
Screenshot 2023-12-25 205423

@HugoFara
Copy link
Owner

For the SYSTEM_ADMIN_VARIABLES, I think it will be better to rewrite the portions of code concerned in a clean way, we can discuss it there: #167 .

For the TTS (Text-To-Speech) interactions, would you like to implement the new system of options? So that I can work on fixes on meantime and pull your request when it's ready.

@ProgramComputer
Copy link
Author

For the SYSTEM_ADMIN_VARIABLES, I think it will be better to rewrite the portions of code concerned in a clean way, we can discuss it there: #167 .

For the TTS (Text-To-Speech) interactions, would you like to implement the new system of options? So that I can work on fixes on meantime and pull your request when it's ready.

The new way of options is added now. install.sh changes were removed too.

Copy link
Owner

@HugoFara HugoFara left a comment

Choose a reason for hiding this comment

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

Thanks for your changes, it's all good for me!

@HugoFara HugoFara merged commit e764e9a into HugoFara:dev Dec 30, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Develop an existing feature invalid This doesn't seem right ux User Experience could be better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants