After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 767394 - Migrate GtkTable to GtkGrid and add CSS selectors
Migrate GtkTable to GtkGrid and add CSS selectors
Status: RESOLVED FIXED
Product: gnome-calculator
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gcalctool maintainers
gcalctool maintainers
Depends on:
Blocks:
 
 
Reported: 2016-06-08 13:24 UTC by Niels De Graef
Modified: 2016-10-11 07:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch - GtkTable->GtkGrid and add some CSS classes (115.71 KB, patch)
2016-06-08 13:24 UTC, Niels De Graef
none Details | Review
Added extra CSS classes (19.93 KB, patch)
2016-06-10 13:15 UTC, Niels De Graef
none Details | Review
Screenshot of financial mode (47.47 KB, image/png)
2016-09-16 11:18 UTC, Robert Roth
  Details
Patch - Fixed row-spacing in buttons-financial.ui (850 bytes, patch)
2016-09-16 11:32 UTC, Niels De Graef
committed Details | Review
Patch - GtkTable->GtkGrid and add some CSS classes (applicable to master) (115.44 KB, patch)
2016-10-04 11:15 UTC, Niels De Graef
committed Details | Review
Patch - Added extra CSS classes (applicable to master) (19.94 KB, patch)
2016-10-04 11:21 UTC, Niels De Graef
committed Details | Review

Description Niels De Graef 2016-06-08 13:24:19 UTC
Created attachment 329382 [details] [review]
Patch - GtkTable->GtkGrid and add some CSS classes

GtkTable has been deprecated for a while and has been replaced with GtkGrid.
This change:
* reduces the amount of warnings in Glade
* allows for easier design changes (e.g. changing a bit marker's color now only needs one extra line in calculator.css)
* Which in its own makes it easier to solve other bugs such as bug 756920, by allowing extra/easier styling.

If you want, I can continue migrating code.
Comment 1 Niels De Graef 2016-06-10 13:15:27 UTC
Created attachment 329572 [details] [review]
Added extra CSS classes

Also added CSS classes for
* numeric buttons (0..F)
* the numeric point button
* the fx-button
Comment 2 Robert Roth 2016-09-16 11:15:11 UTC
Review of attachment 329382 [details] [review]:

Looks fine, only buttons in Financial mode do not have any vertical padding, and are placed edge-by-edge, this should be fixed.
Comment 3 Robert Roth 2016-09-16 11:18:04 UTC
Created attachment 335693 [details]
Screenshot of financial mode

See a screenshot with Calculator 3.20 (left) and Calculator master with the patch applied (right). You can see that the buttons are touching their vertical neighbours.
Comment 4 Niels De Graef 2016-09-16 11:32:12 UTC
Created attachment 335694 [details] [review]
Patch - Fixed row-spacing in buttons-financial.ui

Whoops, good catch! Should be fixed with this trivial patch.
Comment 5 Robert Roth 2016-09-16 12:19:10 UTC
Ok, with this it works fine. I would say we should go with these patches after the stable release is out.
Comment 6 Michael Catanzaro 2016-09-16 16:27:35 UTC
Yeah, no freeze exception for this. Thanks for helping out with Calculator, both of you!
Comment 7 Robert Roth 2016-09-25 01:41:56 UTC
The patches do not apply due to some recent styling changes from commit [1], could you please update the problematic patch? It would help this happen after branching gnome-3-22, I think most maintainers do that after 3.22.1 is out.
[1] https://git.gnome.org/browse/gnome-calculator/commit/?id=bad0f883d6c086d90ca2e07991dbe2d76de9aa24
Comment 8 Robert Roth 2016-10-04 06:18:06 UTC
@Niels De Graef: do you have the time to update the patches this week, to get the changes in master early in the cycle? If not, just let us know, and we'll do our best to update the patches and include them, if possible.
Comment 9 Niels De Graef 2016-10-04 11:15:32 UTC
Created attachment 336893 [details] [review]
Patch - GtkTable->GtkGrid and add some CSS classes (applicable to master)

Reworked patch. Been a bit busy since the academic year started again, but reworking a patch shouldn't be a problem. :)
Comment 10 Niels De Graef 2016-10-04 11:21:27 UTC
Created attachment 336894 [details] [review]
Patch - Added extra CSS classes (applicable to master)
Comment 11 Robert Roth 2016-10-11 07:01:13 UTC
Thanks for the contribution, gnome-3-22 is branched, your fixes are on master now, due to be released
with the next development version.