Resource Download - Settings - #1305
Conversation
|
|
||
| public: | ||
| inline bool get_enabled(){ return m_enabled; } | ||
| inline void set_enabled(bool enabled){ |
There was a problem hiding this comment.
Are you trying to fade out the button to lock it? Even ConfigOption has a visibility field. ConfigOption::set_visibility() will set it and will automatically show/hide/grey-out the entire option unless the subclass explicitly overrides it.
There was a problem hiding this comment.
I forgot about set_visibility() when I wrote this. But to be fair, I'm not only fading the button, but also changing the button text. So, I'm not sure if refactoring to use set_visibility() is worth it.
There was a problem hiding this comment.
This is where it gets used:
void SettingsDownloadButtonWidget::update_UI_state(){
if (m_value.get_enabled()){
m_button->setEnabled(true);
m_button->setText("Download");
}else{
m_button->setEnabled(false);
if (m_row.is_given_action_state(ActionState::PRE_DOWNLOAD)
|| m_row.is_given_action_state(ActionState::DOWNLOADING))
{
m_button->setText("Downloading...");
}
}
}
There was a problem hiding this comment.
There's 2 ways to do this. You can either override set_visibility() and have it immediately call the parent to retain the visibility behavior, then you can add more stuff to it. Or you can call it manually along side a separate setText call.
Calling set_visibility() will go through the listener system to notify everything that's listening for this event. So you'll want to not bypass that.
|
This PR should now be ready for review. |
| fs::remove_all(Filesystem::Path(resource_directory)); | ||
|
|
||
| // update the table labels | ||
| set_is_downloaded(false); |
There was a problem hiding this comment.
What happens if something happens after the files are deleted, but before set_is_downloaded(false) is called? Can it leave the program or resource folder in a bad state?
There was a problem hiding this comment.
set_is_downloaded() is only to update the table labels. The main thing is ensuring that the buttons aren't frozen. update_action_state() is what unblocks the buttons. And it runs even when exceptions are thrown.
There was a problem hiding this comment.
Thinking about this more, I think I'll use ScopeExit to ensure that update_action_state() is always run, and to avoid repeating myself.
| PA_ADD_OPTION(USE_PADDLE_OCR); | ||
| PA_ADD_OPTION(USE_GPU_FOR_ML_INFERENCE); | ||
| PA_ADD_OPTION(RESOURCE_DOWNLOAD_TABLE); | ||
| PA_ADD_OPTION(DOWNLOAD_ERROR); |
There was a problem hiding this comment.
Wondering if we should gate this table behind dev mode for now.
2 participants
No description provided.