Skip to content

Gmoccapy: Move tool table buttons to table frame, add toggle button for calculator use #3509

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Sigma1912
Copy link
Contributor

tooledit classic
tool edit material

@Sigma1912
Copy link
Contributor Author

Auto scroll to the bottom of the tool list when adding new tool:
Recording

@Sigma1912 Sigma1912 mentioned this pull request Jul 7, 2025
@hansu
Copy link
Member

hansu commented Jul 9, 2025

[Norbert]

Hallo,

IMHO we should delete the column with the checkboxes. The checkbox column is an historical relict. Long time ago, when I coded that widget, I have not been able to find a solution to edit cells without the checkbox. And also multiple line selection was not possible at that state. Sure some of the problems are caused by my poor python knowledge at that time.

Is it possible to select several lines holding down and clicking that line, as we all are used to do in Excel? That would allow deleting several tools at once (I can not remember having done that in all my time working on CNC machines).

The single selected line will be edited, if several lines have been selected, which one will be used for the next step? That is why you get an error if you change tool and have not selected any row or several.

Norvbert

@hansu
Copy link
Member

hansu commented Jul 9, 2025

[Sigma1912]

@hansu We had a small discussion about the tool table and using 'switch to highlighted' instead of 'switch to checked' tool somewhere. I can't find it, but I was wondering if the 'Select'/'Auswählen' column could be 'Delete'.

Sounds good.

But if only using that column for deleting - then I don't see in the tool table which tool is actually in the spindle after I changed the selection.

If keeping the checkbox, it makes clear that I am changing from tool 6 to 8:
grafik

Of course I can see the current tool in the Tool information area, but only if that is visible.
Or can we format the line of the current tool in bold?

And I would keep the four buttons left aligned - it looks a bit odd so.

@hansu
Copy link
Member

hansu commented Jul 9, 2025

And For the save button I would suggest the check button like mentioned in #2818 (comment).
Yes it does a save action but also tells LinuxCNC about the changed tool table, so a check button would be mor intuitive I think.
If we are really fancy, we create a combined button.

@Sigma1912
Copy link
Contributor Author

But if only using that column for deleting - then I don't see in the tool table which tool is actually in the spindle after I changed the selection.

Note that if the loaded tool and the tool to be changed to are far enough apart in you don't actually see the checkbox anymore.

Current tool loaded would be displayed in the button box:
current tool

@Sigma1912
Copy link
Contributor Author

And I would keep the four buttons left aligned - it looks a bit odd so.

I want to keep buttons that change the table to the left and buttons that dont to the right. Pulling the ones on left apart gives more room for error when using a touch screen.

@hansu
Copy link
Member

hansu commented Jul 9, 2025

Note that if the loaded tool and the tool to be changed to are far enough apart in you don't actually see the checkbox anymore.

That is true

Current tool loaded would be displayed in the button box:

👍

That's a good solution. But I would move the tool display to the left to be closer to the tool-nr column:

grafik

@Sigma1912
Copy link
Contributor Author

And For the save button I would suggest the check button like mentioned in #2818 (comment).
Yes it does a save action but also tells LinuxCNC about the changed tool table, so a check button would be mor intuitive I think.

Maybe we should just replace the yellow arrow on the 'back' button with the green check mark? I'm not overly keen on having another button to leave with accepting unfinished entries.
One of the things that have always bugged me was the label 'Apply' which is not the same as 'Save', we might want to indicate if the table has not been saved on leaving the page.

@Sigma1912
Copy link
Contributor Author

Also we need to decide whether we want to keep the checkbox column for deleting multiple tools or remove it as norbert has suggested.

@hansu
Copy link
Member

hansu commented Jul 9, 2025

Maybe we should just replace the yellow arrow on the 'back' button with the green check mark?

Hmm you might want to have an abort option, if you accidentally removed some tools.

One of the things that have always bugged me was the label 'Apply' which is not the same as 'Save', we might want to indicate if the table has not been saved on leaving the page.

We can add a warning dialog like when exiting from the G-code editor when having changes?

@Sigma1912
Copy link
Contributor Author

Hmm you might want to have an abort option, if you accidentally removed some tools.

As long as the table has not been saved you can just click on the reload button. Even if you clicked on not saving in the proposed popup on leaving the page,
I think we should add an ESC key handler to abort editing of a cell, seems intuitive to expect that to work.

@gmoccapy
Copy link
Collaborator

One question after a very very short look at the code changes:
Within tooled it widget the "new button line" is created. So the widget could be used also stand allone. But in the gmoccapy.py file the buttons from the widget are hidden and created new in gmoccapy. IMHO the buttons from the widget should send a signal and gmoccapy should just react to that signal.

Sorry, if I haven't check the code enaugh and beeping wrong here.

Norbert

@hansu
Copy link
Member

hansu commented Jul 10, 2025

I only wonder why you remove the buttons inside the widget's buttonbox and create new ones. Does it not work to change only the labels to icons?

@Sigma1912
Copy link
Contributor Author

I'll see what I can do to reuse more of the existing code.

@Sigma1912 Sigma1912 force-pushed the Gmoccapy_Add_buttons_to_tooledit_frame branch from 8d07a42 to b553a29 Compare July 13, 2025 08:30
@Sigma1912
Copy link
Contributor Author

How about adding a settings option to show/hide 'Select' column given that selecting multiple rows with a touchscreen only is inconvenient. If a keyboard is used the CTRL/SHIFT keys could be used to select multiple rows and the 'Select' column is not required.

@hansu
Copy link
Member

hansu commented Jul 14, 2025

Sure we can add a setting to hide the select column. But I don't really get the point why - Is it mostly to avoid confusion? Or space reasons?

I agree that this column is now only used in a rare use case - deleting multiple tools by mouse or touch only.
We can also use a toggle button for deleting. So every selection with an active delete button will remove the line/tool. But might be also a bit unexpected.


What do you think of highlighting the active tool by a blue-ish color like it is done in the offset page widget?

@hansu
Copy link
Member

hansu commented Jul 14, 2025

I mentioned resp. asked recently somewhere (didn't find now where it was) if it's the best strategy to change the buttons of the widgets like this one in the gmoccapy code and not in the widget itself.
I think it would be better for maintainability if this is done in the widget's code. Sure, this will affect other GUIs like Gscreen which use these widgets, but maybe they benefit also from these changes. @c-morley what do you think?

@c-morley
Copy link
Collaborator

If buttons makes sense for general use, (I didn't read back enough to see what buttons you are referring to) then I would think adding them to the widget would be better.
Similarly you could add a function that would be used to add a button to the existing button box, so a third party could add whatever buttons they want.

If you must change all the buttons for some reason, then adding a function or property to hide/delete the button box and (possibly) add a new button box with new specified buttons would seem reasonable.

@hansu
Copy link
Member

hansu commented Jul 15, 2025

If buttons makes sense for general use, (I didn't read back enough to see what buttons you are referring to) then I would think adding them to the widget would be better.

It's more about changing the existing buttons to have an icon instead of just a label and change the size and maybe the order.

@Sigma1912
Copy link
Contributor Author

Sure we can add a setting to hide the select column. But I don't really get the point why - Is it mostly to avoid confusion? Or space reasons?

It's to avoid confusion. The only use case for the Select column is to select multiple tools with a touch screen w/o a keyboard.

@c-morley
Copy link
Collaborator

If buttons makes sense for general use, (I didn't read back enough to see what buttons you are referring to) then I would think adding them to the widget would be better.

It's more about changing the existing buttons to have an icon instead of just a label and change the size and maybe the order.

Can that not be controlled with css styles?

@hansu
Copy link
Member

hansu commented Jul 16, 2025

I don't think we can add an icon with CSS...

@Sigma1912
Copy link
Contributor Author

What do you think of highlighting the active tool by a blue-ish color like it is done in the offset page widget?

This would require a hidden column added to the tool data with the 'foreground' color. At least that is how it is done in the offset table and I don't know of an other way to change the text color. This column would need to be inserted when reading the tool table file and would need to be ignored when writing to the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants