Skip to content
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

Replaced Gtk::Alignment spaces with CSS margins #2195

Merged
merged 6 commits into from Jun 28, 2021

Conversation

DhairyaBahl
Copy link
Member

@ice0 @Keyikedalube @rodolforg

This PR is meant for removal of SPACES that are added with the help of alignment.h .

@DhairyaBahl
Copy link
Member Author

DhairyaBahl commented Jun 20, 2021

I've removed alignment.h from 2 states ! Kindly tell me if this is the correct way.

Thanks :)

@Keyikedalube
Copy link
Contributor

They're correct.

Instead of defining GAP & INDENTATION in multiple files, this can also be done in CSS too:

#gap {
    margin-end: 3px
}

And calling widget->set_name("gap")

@DhairyaBahl
Copy link
Member Author

DhairyaBahl commented Jun 21, 2021

@Keyikedalube After referring to the documentation, and after trying gtkmm/cssprovider.h tried adding css file to the state_bline but it showed following error . Kindly tell me what I am doing wrong in this.

what I did ?

I just initialised provider , screen and then imported css file and stored it in css_file . and this is showing error in these lines which you can see in the screenshot. I tried debugging them but I can't solve the issue. Kindly help.

Screenshot (17)

Thanks :)

@Keyikedalube
Copy link
Contributor

@DhairyaBahl Synfig already loaded its CSS file at startup. You can find the source at synfig-studio/src/gui/resources/css

You only need to add the CSS code statement in the existing CSS file and call the CSS id from widget that needs spacing.

@ice0
Copy link
Collaborator

ice0 commented Jun 21, 2021

This spacing is between "Name:" label and text box.

Screenshot_123

Since they are already packed into a Gtk::Box widget, the native way is to set the spacing property (https://developer.gnome.org/gtkmm/stable/classGtk_1_1Box.html#ab5396cb1385f8259bb49a0c78b55087b)

But to make this spacing consistent for all tools (state_... classes are used to describe Synfig tools), the best way is to add class to this Box widget, or set they name/id and change spacing through CSS as @Keyikedalube suggested.

@DhairyaBahl
Copy link
Member Author

Synfig already loaded its CSS file at startup. You can find the source at synfig-studio/src/gui/resources/css

Understood ! BTW i was creating new css file ! but console was showing errors that are given above . is it not possible to create multiple css files ? if it is possible then kindly tell me what mistake I was doing with that file ?

But to make this spacing consistent for all tools (state_... classes are used to describe Synfig tools), the best way is to add class to this Box widget, or set they name/id and change spacing through CSS as @Keyikedalube suggested.

Understood [^_^] . Thank You !

@DhairyaBahl
Copy link
Member Author

DhairyaBahl commented Jun 21, 2021

@Keyikedalube I've made the required changes in the files but still I am unable to see those changes in the app ! There is no spacing now ! trying to find out what went wrong !

And secondly we are doing margin-end : 3px for #gap but margin end is not the property of css ! It have margin-right or margin-left ! Then why is it working ?

Thank You :)

@ice0
Copy link
Collaborator

ice0 commented Jun 21, 2021

And secondly we are doing margin-end : 3px for #gap but margin end is not the property of css ! It have margin-right or margin-left ! Then why is it working ?

This is a right question. It don't.
If you increase margin-end to 30px, you will see no changes. But you will see the warning in console when synfigstudio starts:

synfig(15114) [10:48:38 AM] warning: synfig.css:90:11'margin-end' is not a valid property name

after replacing it with margin-right - it works fine now.

Also I noticed where is no CSS spacing property for Gtk::Box.
P.S. You can check all available CSS properties for widget with GTK Inspector.

@Keyikedalube
Copy link
Contributor

@DhairyaBahl In CSS code it's margin-left/right and in GTK code it's margin-start/end

Simple, change it to left/right and your CSS code would work :)

@Keyikedalube
Copy link
Contributor

BTW i was creating new css file ! but console was showing errors that are given above . is it not possible to create multiple css files ? if it is possible then kindly tell me what mistake I was doing with that file ?

It's possible but you're trying to add/override another "global" CSS provider which was already done in main Synfig initialization.
The correct member function you need is add_provider(...) but that's just redundant if you go that route because you're going to be adding more lines of CSS loading code and then applying it to widgets over multiple state_*.cpp files.
We already loaded and applied the global CSS provider so use them.

@DhairyaBahl
Copy link
Member Author

The correct member function you need is add_provider(...) but that's just redundant if you go that route because you're going to be adding more lines of CSS loading code and then applying it to widgets over multiple state_*.cpp files.

Okayy @Keyikedalube understood [^_^] .

@Keyikedalube @rodolforg @ice0
I've removed deprecated alignment.h from all the states ! Kindly review this PR ! Will now look for other deprecated classes or check if alignment.h is still left at some parts and will replace them accordingly.

Thanks and regards !

@ice0
Copy link
Collaborator

ice0 commented Jun 22, 2021

@DhairyaBahl Please remove

#define GAP	(3)

and

const int GAP = 3;
const int INDENTATION = 6;

as they not needed anymore.

Also after looking at the whole PR, it seems to me that it is better to replace the widget->set_name("gap") with
widget->add_class("gap") (and #gap -> .gap) since it better reflects what we want to do (same for indentation).

@DhairyaBahl
Copy link
Member Author

@ice0 I tried doing the same but it is showing error that no member named add_class This shouldn't be happening coz add_class is a valid syntax in gtk as well ! Then , why is it happening ?

@ice0
Copy link
Collaborator

ice0 commented Jun 23, 2021

@ice0 I tried doing the same but it is showing error that no member named add_class This shouldn't be happening coz add_class is a valid syntax in gtk as well ! Then , why is it happening ?

Ok, now I see the problem.
add_css_class is available from GTK4. GTK3 recommends to use set_name (https://developer.gnome.org/gtkmm/3.24/classGtk_1_1Widget.html#a12b052fca2ace42902e0448862fbf189) to refer widget from CSS file.

So the only thing to do here is to remove redundant variables.

P.S. To check the documentation for GTK3, just replace stable with 3.24 in the documentation url. (https://developer.gnome.org/gtkmm/stable/ to https://developer.gnome.org/gtkmm/3.24/)

@rodolforg
Copy link
Contributor

You can add a class by:
get_style_context().add_class() .

@DhairyaBahl
Copy link
Member Author

@rodolforg @ice0 It is still giving this error

image

Although its written in the docs that add_class is a valid method of styling ! But still it is not working ! You can see in the picture, how I am doing it ! Kindly tell me what am i doing wrong ?

Thankks :)

@Keyikedalube
Copy link
Contributor

Although its written in the docs that add_class is a valid method of styling ! But still it is not working ! You can see in the picture, how I am doing it ! Kindly tell me what am i doing wrong ?

You can see in screenshot Gtk::StyleContext is wrapped by a reference pointer so the correct operator to access its member function is -> not .
Also, one cannot access object member using dot . if a variable is a pointer too.

@DhairyaBahl
Copy link
Member Author

@ice0 I haven't removed the gap constant coz it is being used multiple times in some files ! Kindly tell me if I have to remove it too and enter the values manually ! I will do it asap ! Thanks :)

@ice0
Copy link
Collaborator

ice0 commented Jun 25, 2021

@ice0 I haven't removed the gap constant coz it is being used multiple times in some files ! Kindly tell me if I have to remove it too and enter the values manually ! I will do it asap ! Thanks :)

You are right, they are still needed. So I don't see any other issues here. Good work!

@ice0 ice0 requested a review from rodolforg June 26, 2021 00:01
@ice0 ice0 changed the title Removing the spaces that are added using alignment.h Replaced Gtk::Alignment spaces with CSS margins Jun 28, 2021
@ice0 ice0 merged commit c3e196e into synfig:master Jun 28, 2021
@ice0 ice0 added the GSoC Gsoc Idea/Proposal label Jun 28, 2021
@ice0
Copy link
Collaborator

ice0 commented Jun 28, 2021

Merged. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC Gsoc Idea/Proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants