feat: added ability to show/hide rulers

[mohamedAdhamc]

as asked for in #1077
whats left to do:
1- currently there is no icon for the ruler so I will have to make one myself im guessing.
2- probably would be useful to also add this in the menu as is done for the other similar buttons.

this is a how it works as of now
before:

after:

[ice0]

@mohamedAdhamc

1. You have a memory leak here. Please read the comments on this PR fix: memory leak when the load default palette button is repeatedly pressed #2633 to see why.
2. Why not just show/hide the rulers here instead of deleting and creating them over and over again?
3. Try not to duplicate code. If someone wants to change something, he will have to find all these places.

[mohamedAdhamc]

Hey @ice0 , thanks for the review
Regarding
1- I had a feeling that this method of implementation could possibly cause problems, I will look into that for sure.

2- problem is that this features goal I believe was to save space, So one would have to not only hide the ruler but also the corresponding grid row/column. I kept looking for a workaround to not have to remove and reinsert the grids rows/colums something like a hide()/show() for the gtk grid columns/rows but there weren't any. So the only way was to manually remove and reinsert the specified rows/columns. Then just reattach the ruler widgets along with the arrow box which I recreated and caused the memory leakage here I'm assuming as its created each time without being deleted.

3- yes the duplication here is surely not good I will try to make the gtk::event box a class member then so I wouldnt have to recreate it. Although that requires adding the event box header in the workarea header, but yes I guess it would be worth it, to avoid the duplication.

Update: removed duplicated code which contained repeated creation of the menu box, I believe now the memory leakage should probably be fixed am I correct ?
Now the working of the button is just detaching the rulers and menu box first then removing the row. And then when pressed again re-inserting the row and re-attaching the detached widgets.

[ice0]

@mohamedAdhamc

I kept looking for a workaround to not have to remove and reinsert the grids rows/colums something like a hide()/show() for the gtk grid columns/rows but there weren't any.

Row/column will hide if you hide all widgets in them.

[mohamedAdhamc]

Row/column will hide if you hide all widgets in them.

Oh that's great actually, then that's what needs to be done.

[ice0]

1. Looks good, but you can simplify the code even further:

	ruler_status = !ruler_status;
	hruler->set_visible(ruler_status);
	...

2. I suggest renaming the method to WorkArea::show_hide_ruler() to better reflect its purpose.
3. I've noticed that Gtk::Arrow is deprecated, so I suggest replacing it with Gtk::Button (in a separate PR).

[ice0]

1- currently there is no icon for the ruler so I will have to make one myself im guessing.

If you'd like, you can ask the artists on the Synfig forums for help.

I also suggest saving the state of the rulers in the settings file.

[mohamedAdhamc]

1. Looks good, but you can simplify the code even further:

	ruler_status = !ruler_status;
	hruler->set_visible(ruler_status);
	...

2. I suggest renaming the method to WorkArea::show_hide_ruler() to better reflect its purpose.
3. I've noticed that Gtk::Arrow is deprecated, so I suggest replacing it with Gtk::Button (in a separate PR).

All sounds good, will be working on them tonight hopefully. Also yes, thats a great idea I will in the meantime ask in the forums, that way we could also get some opinions about the icons I guess.

[mohamedAdhamc]

hey, @ice0
Now the setting is saved so when the ruler is hidden in a canvas and the canvas is reopened, it remains hidden and vice versa. Also default for creating new canvas is with ruler shown, which is the regular of course. I'm assuming this is what you meant yesterday by saving the ruler setting, if not though please let me know.

as of right now I believe whats left is:

• icon
• the menu check button for the feature, if it is necessary I'm assuming it is...?
• replacement of deprecated Gtk::Arrow but that's going to be a different pull request 😀.

update: oh yes and also the tool tip

[mohamedAdhamc]

Update: now the check button is added and its synced with the tool button. and the meta data is saved correctly so the ruler position is left as is when reloading the saced canvas.
yet to do :
1- add a ruler logo for the tool button
2- and of course the tooltip

[mohamedAdhamc]

Icon and tooltip added. However, I think maybe the icon color needs to be greyer. So i'll just wait for you guy's opinionson this and if so I would just place the icon with the new edited verision.

update: Woah just realized the .sif file is huge it's around 9000 lines, while for comparison the show grid icon is about 3000. so definitely I need to remove some things to make it simpler.

this is how it looks now

[ice0]

@morevnaproject

Do we need an explicit button to toggle the rulers, or would a menu item suffice?
I mean I don't see any reason to overload the UI.

P.S. The original feature request is also for the menu item only. (#1077)

[mohamedAdhamc]

Hey, @ice0 so should I remove the button, and just leave the menu item ?

[ice0]

Hey, @ice0 so should I remove the button, and just leave the menu item ?

I suggest to remove the button, we can add it later if users need it.

[mohamedAdhamc]

I suggest to remove the button, we can add it later if users need it.

sounds good will do

[mohamedAdhamc]

done, I also retested it to make sure the meta data is saved correctly on reopening the file.

[mohamedAdhamc]

@ice0, done. Is anything else left here or is it good to go?

[mohamedAdhamc]

@rodolforg done

[rodolforg]

Almost there :)

[rodolforg]

@ice0 We need to squash before merge

[ice0]

Merged. Thank you!

[mohamedAdhamc]

Merged. Thank you!

hopefully many more merges to come :)