[WIP] GUI #43
[WIP] GUI #43
Conversation
|
before I spend a long time making a copy function for every single text translation option is there any more effective way to do that where it will still work? |
why do you need to create a copy function for every single option tf |
|
also why are you doing this on your fork |
…d the non-GUI version of this
Because it took last steps for me to figure out (remember I don't know how to use GitHub property)
Because I if I try and get the variable within the button and not in a function then it doesn't work and will just give me nothing |
|
I believe that this is getting to the point where it's usable now (other than the fact that it won't build) |
|
lemme go fix the workflow rq |
adding to release workflow later
|
I believe this works now |
There was a problem hiding this comment.
You could probably shorten this code down drastically but it's not a hard requirement to merge the PR, after all a bit of spaghetti code was done myself in the early days of translator, plus we can always refactor it later if need be.
If you would like to improve the code before merging, examples are below. Otherwise, feel free to undraft the PR and merge it after giving the artifacts built by GitHub Actions a test.
| input_widget = ttk.Entry(root, textvariable=translate_text) | ||
| input_widget.grid(row=0, column=0, columnspan=2) | ||
|
|
||
| ttk.Label(text="Reversed:").grid(row=1,column=text_lable_column) |
There was a problem hiding this comment.
Starting from this line, you're copy pasting a lot over per label, which is unnecessary. You could put this in a while loop since they seem to all be the same except for variable names, text labels and rows
There was a problem hiding this comment.
So how would I get it to change the variable name?
| emoticons_text['text'] = converter.text_to_emoticons(new_text) | ||
|
|
||
| def add_copy_buttons(): | ||
| ttk.Button(root, command=partial(copy_to_clipboard, reversed_text), image = copy_icon).grid(sticky = W, row=1, column=copy_button_column) |
There was a problem hiding this comment.
You could simplify this using loops, arguably simpler than later in the code to do so.
| new_text = translate_text.get() | ||
| x_size=int(8.5*len(new_text)+240) | ||
| root.geometry(f"{x_size}x{350}") | ||
| reversed_text['text'] = converter.reverse_text(new_text) |
There was a problem hiding this comment.
these could all be done using dictionary probably but I'm not sure about capitalized and lowercase text
There was a problem hiding this comment.
I currently do not know how to use a Dictionary in python
|
I think I selected the wrong lines to demonstrate what I mean because I'll be honest I don't review PRs that much, if you need some clarifications or pointers feel free to ask on Discord :) In the meantime I'll be testing the files built by CI |
|
change of plans: do NOT merge this PR yet |
No description provided.