Generate settings tabs for each talker #430

mizaki · 2023-02-11T01:20:46Z

Mostly based on the previous work. Changed passing in the current talker to the whole list for the settings window.

lordwelch

I think I also want to do some extra processing for the API key but I'm not sure what.

comictaggerlib/settingswindow.py

mizaki · 2023-02-14T01:52:57Z

Possibly a lot of issues:

File location. Seemed to make sense as it was UI related but...?
File name?
Should the menu generator be a class?
I feel like there should be an easier way with the test api/url functions (making the whole thing a class?).
Adding separate functions for checking API and URL made sense to me.
Not sure on the benefit of moving the specific widget generator function out on their own? (Probably missing something.)

comictaggerlib/ui/talkeruigenerator.py

comictaggerlib/ui/settingswindow.ui

comictaggerlib/settingswindow.py

Rename test_api_key and test_api_url to api_key_btn_connect and api_url_btn_connect Make separate function to set form values, called in settings_to_form Change isinstance to is Call findChildren only once

…earch tab.

mizaki · 2023-02-18T17:46:01Z

That should have addressed everything. General for talkers is looking a little bare now it only have the dropdown for the talker selection.

comictaggerlib/ui/talkeruigenerator.py

tests/talker_test.py

comictaggerlib/settingswindow.py

…dow. Additional clean up.

…tting and add to sources dict of widgets.

lordwelch

The last thing is I really don't like how the api key and URL are setup, it doesn't make sense to test the url separate from the API key, I know it makes generating the UI easier but I still don't like it. I'm planning on pushing a commit today to make it consistent across all talkers.

comictaggerlib/ui/talkeruigenerator.py

mizaki · 2023-02-21T00:36:49Z

The last thing is I really don't like how the api key and URL are setup, it doesn't make sense to test the url separate from the API key, I know it makes generating the UI easier but I still don't like it. I'm planning on pushing a commit today to make it consistent across all talkers.

It's not so much about making it easier to generate, more that most APIs don't require a key so there will only be a URL to test. On that note, it looks like you've made every talker have an API key entry?

# Conflicts: # comictaggerlib/settingswindow.py # comictalker/talkers/comicvine.py

comictaggerlib/ui/talkeruigenerator.py

comictaggerlib/ui/settingswindow.ui

comictaggerlib/ui/talkeruigenerator.py

Generate API text field in their own function API tests return string message of result Add help to text field lables

lordwelch

When these last few comments are addressed I'll merge this.

comictaggerlib/ui/talkeruigenerator.py

comictalker/talkers/comicvine.py

comictaggerlib/ui/talkeruigenerator.py

Change metadata select label Use named tuple for talker tabs Retrun a string and bool for api check

mizaki · 2023-02-24T00:10:44Z

The final "Make it return a tuple[str, bool]" didn't make sense to me for line 201?

lordwelch · 2023-02-24T02:47:55Z

The final "Make it return a tuple[str, bool]" didn't make sense to me for line 201?

Right now it is only used to give a message in the GUI, but if it is ever to be used elsewhere I need to be able to determine if it is successful or not. With only a string I cannot determine if the test was successful or not.

lordwelch · 2023-02-24T08:04:49Z

Now this extra commit needs to be removed aba59bd and I can merge

This reverts commit aba59bd.

mizaki · 2023-02-24T13:26:12Z

The final "Make it return a tuple[str, bool]" didn't make sense to me for line 201?

Right now it is only used to give a message in the GUI, but if it is ever to be used elsewhere I need to be able to determine if it is successful or not. With only a string I cannot determine if the test was successful or not.

The one for the API test I understood. It was the same comment for

for option in config[1][f"talker_{talker_id}"][1].values():
            current_widget = None

that had me confused. I wondered if it was a c/p mistake from the API one?

mizaki · 2023-02-24T13:27:07Z

Now this extra commit needs to be removed aba59bd and I can merge

Not sure how that got in there. Must have been from the merge somehow. Reverted it.

lordwelch · 2023-02-28T06:36:55Z

I wondered if it was a c/p mistake from the API one?

No idea how it got there???

Generate settings tabs for each talker

83a8d5d

lordwelch requested changes Feb 11, 2023

View reviewed changes

mizaki added 2 commits February 14, 2023 01:32

Move talker settings menu generator to a separate file

6a6a332

Test for menu generator format_internal_name

2fde11a

lordwelch requested changes Feb 17, 2023

View reviewed changes

mizaki added 2 commits February 18, 2023 01:15

Use None as parent for api and url message boxes

02fd8be

Rename test_api_key and test_api_url to api_key_btn_connect and api_url_btn_connect Make separate function to set form values, called in settings_to_form Change isinstance to is Call findChildren only once

Generate talker general tab programatically. Move search options to s…

a24bd1c

…earch tab.

lordwelch requested changes Feb 19, 2023

View reviewed changes

mizaki added 3 commits February 19, 2023 23:33

Remove general tab from talker tab and use base tab from settings win…

fefb3ce

…dow. Additional clean up.

Add another test case for format_internal_name

bd5e23f

Use new display_name from settngs. Add source combobox getting and se…

f439797

…tting and add to sources dict of widgets.

lordwelch requested changes Feb 20, 2023

View reviewed changes

mizaki and others added 5 commits February 21, 2023 00:38

Fix optoin.type ifs

59893b1

Use currentData for combo box

316bd52

docs(contributor): contrib-readme-action has updated readme

aba59bd

Merge branch 'develop' into talker_settings_generator

33ea8da

# Conflicts: # comictaggerlib/settingswindow.py # comictalker/talkers/comicvine.py

Fix api key test button generation

5b5a483

lordwelch reviewed Feb 21, 2023

View reviewed changes

comictaggerlib/ui/talkeruigenerator.py Outdated Show resolved Hide resolved

comictaggerlib/ui/talkeruigenerator.py Outdated Show resolved Hide resolved

comictaggerlib/ui/settingswindow.ui Outdated Show resolved Hide resolved

lordwelch reviewed Feb 22, 2023

View reviewed changes

comictaggerlib/ui/talkeruigenerator.py Show resolved Hide resolved

Change source term to metadata

118429f

Generate API text field in their own function API tests return string message of result Add help to text field lables

lordwelch approved these changes Feb 23, 2023

View reviewed changes

Add missing tool tips to labels

b4a3e8c

Change metadata select label Use named tuple for talker tabs Retrun a string and bool for api check

Revert "docs(contributor): contrib-readme-action has updated readme"

2611c28

This reverts commit aba59bd.

lordwelch merged commit fca5818 into comictagger:develop Feb 28, 2023

lordwelch mentioned this pull request Jun 3, 2024

Add remove HTML tables back #653

Merged

Generate settings tabs for each talker #430

Generate settings tabs for each talker #430

Uh oh!

Conversation

mizaki commented Feb 11, 2023

Uh oh!

lordwelch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mizaki commented Feb 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mizaki commented Feb 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lordwelch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mizaki commented Feb 21, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lordwelch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mizaki commented Feb 24, 2023

Uh oh!

lordwelch commented Feb 24, 2023

Uh oh!

lordwelch commented Feb 24, 2023

Uh oh!

mizaki commented Feb 24, 2023

Uh oh!

mizaki commented Feb 24, 2023

Uh oh!

lordwelch commented Feb 28, 2023

Uh oh!

Uh oh!