Skip to content

Disable WF case buttons if Case Ignore #291

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

Merged
merged 3 commits into from
Jun 22, 2024

Conversation

windymilla
Copy link
Collaborator

When Case Ignore is on, words that are ALL CAPS, Initial Caps, or MiXeD cASe aren't detected, which could
confuse the user. Therefore disable those buttons when Case Ignore is on, and enable them when it's off. Display a tooltip to explain to the user if they are disabled.

Fixes #289

When Case Ignore is on, words that are ALL CAPS, Initial
Caps, or MiXeD cASe aren't detected, which could
confuse the user. Therefore disable those buttons when
Case Ignore is on, and enable them when it's off. Display
a tooltip to explain to the user if they are disabled.
@tangledhelix
Copy link
Member

✅ When setting "ignore case" the buttons are properly disabled, their labels are grayed out, and the tooltips are working.

✅ If you then uncheck "ignore case" the text labels on the buttons go back to normal, and the tooltips disappear.

❌ But, the buttons themselves are still disabled - for all 3 buttons I cannot use that function anymore, unless I close WF and open a new WF dialog.

@srjfoo
Copy link
Member

srjfoo commented Jun 21, 2024

❌ But, the buttons themselves are still disabled - for all 3 buttons I cannot use that function anymore, unless I close WF and open a new WF dialog.

Confirmed. I didn't think to check this in my initial testing. In this round of testing, I tried manually re-running it, but as @tangledhelix says, only closing WF and re-starting it worked.

@windymilla
Copy link
Collaborator Author

❌ But, the buttons themselves are still disabled - for all 3 buttons I cannot use that function anymore, unless I close WF and open a new WF dialog.

Also confirmed (and embarrassed not to have noticed while testing)

Unbind unbinds all when it should only unbind the
function associated with the ID given. Work round
it by getting all bindings, and only reattaching the
ones we want.
@windymilla
Copy link
Collaborator Author

I'm very hesitant to say it's a "python/tkinter/tk/tcl/etc" bug, but in this case it is: python/cpython#75666

So, slightly less embarrassment, and hopefully now fixed. Thanks @tangledhelix for spotting this.

The others were hard to pronounce!
@srjfoo
Copy link
Member

srjfoo commented Jun 21, 2024

Is it supposed to default to All Words if you select "Ignore Case" while one of the case-sensitive options is selected? Right now, everything is deselected for me (macOS). Which I don't have a problem with, but just wanted to make sure that's the expected behavior.

@windymilla
Copy link
Collaborator Author

windymilla commented Jun 21, 2024

For me, it still shows ALL CAPS selected, even though it is disabled, and the list of words is empty. If you hover over it, it displays the tooltip, and if you turn Ignore Case off again, you get back the ALL CAPS words. I'm happy with that behaviour - is that different to what you see?

@tangledhelix
Copy link
Member

tangledhelix commented Jun 21, 2024

For me, it still shows ALL CAPS selected, even though it is disabled, and the list of words is empty. If you hover over it, it displays the tooltip, and if you turn Ignore Case off again, you get back the ALL CAPS words. I'm happy with that behaviour - is that different to what you see?

What you describe is what I see also. You have to look a bit carefully maybe to notice - the disabled widget makes it hard-ish to notice (at least on Mac, at least in Light Mode; I didn't try in Dark Mode).

Screenshot 2024-06-21 at 7 39 49 PM

I think the current behavior is acceptable.

My one gripe is that when checking / un-checking Ignore Case, it's not always obvious that the re-run is currently running and the window freezes up for a couple of seconds (at least on my machine, with my test file, it was a couple of seconds). Auto-triggering a re-run is the correct behavior, but I wonder if there is some non-complex way to indicate the re-run is occurring. I'm not sure what tkinter might have in that regard. It could be as simple as a status label somewhere that switches between two states, e.g. Running ... and Ready. or similar?

An animated graphic (spinner, bar) would be even better but I don't know if there's a widget like that, or if something easy like an animated GIF would render in this context.

animated-gif

@tangledhelix
Copy link
Member

... I did not expect that spinner graphic to be so large! 🤨

@srjfoo
Copy link
Member

srjfoo commented Jun 22, 2024

For me, it still shows ALL CAPS selected, even though it is disabled, and the list of words is empty. If you hover over it, it displays the tooltip, and if you turn Ignore Case off again, you get back the ALL CAPS words. I'm happy with that behaviour - is that different to what you see?

What you describe is what I see also. You have to look a bit carefully maybe to notice - the disabled widget makes it hard-ish to notice (at least on Mac, at least in Light Mode; I didn't try in Dark Mode).

I think the current behavior is acceptable.

That's what I'm seeing now, too. I'm not sure what caused the button last selected not to be selected any longer the time I tried it earlier, but that's not what I'm seeing now.

My one gripe is that when checking / un-checking Ignore Case, it's not always obvious that the re-run is currently running and the window freezes up for a couple of seconds (at least on my machine, with my test file, it was a couple of seconds). Auto-triggering a re-run is the correct behavior, but I wonder if there is some non-complex way to indicate the re-run is occurring. I'm not sure what tkinter might have in that regard. It could be as simple as a status label somewhere that switches between two states, e.g. Running ... and Ready. or similar?

An animated graphic (spinner, bar) would be even better but I don't know if there's a widget like that, or if something easy like an animated GIF would render in this context.

Maybe not as obvious, but would it be possible to force the re-run button to change color as it does when you click on it? It's not quite as obvious on macOS with the default theme when the system is on dark mode as it is on light mode, but there is a difference. Or put an additional drop-shadow on the button while the function is working? (No, please not a spinner of that size! 🤣)

@windymilla
Copy link
Collaborator Author

windymilla commented Jun 22, 2024

I wonder if it would be better to implement a general "busy" state, like GG1 has, which changes the cursor to the busy cursor (whatever that is for your system) and then changes it back when it's done. That seems to be a fairly standard way to indicate that something is happening. Not in this PR of course - I'll add an issue.

@windymilla windymilla merged commit ee6efc7 into DistributedProofreaders:master Jun 22, 2024
1 check passed
@windymilla windymilla deleted the wf-case branch June 22, 2024 20:08
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.

Word Frequency: Initial Caps & Mixed Case don't find anything if Ignore Case is set
3 participants