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 695735 - Convert from GdMainView to EggFlowBox (or something similar)
Convert from GdMainView to EggFlowBox (or something similar)
Status: RESOLVED OBSOLETE
Product: gnome-weather
Classification: Applications
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Weather Maintainer(s)
GNOME Weather Maintainer(s)
Depends on:
Blocks: 695764 696012
 
 
Reported: 2013-03-12 23:12 UTC by Paolo Borelli
Modified: 2014-08-27 14:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Factor out a WorldView class (3.89 KB, patch)
2013-03-12 23:12 UTC, Paolo Borelli
committed Details | Review
Add egg-list-box (2.49 KB, patch)
2013-03-20 18:48 UTC, Giovanni Campagna
none Details | Review
[WIP] Convert from GdMainView to EggFlowBox (19.80 KB, patch)
2013-03-20 18:49 UTC, Giovanni Campagna
none Details | Review

Description Paolo Borelli 2013-03-12 23:12:24 UTC
I think we should try to encapsulate the world view in its own class in preparation to experimenting with using EggFlowBox instead of Gd.MainView (as a matter of fact I would prefer that the WorldView class used composition intead of inheritance and hid the Gd.MainView and the TreeModel to the outside code, but for now this is just the minimal patch).

Incidentally, if instead of setting the model after creating the view, I try the follwing

   this.parent({ model: new WorldModel(world), view_type: Gd.MainViewType.ICON });

then gjs crashes hard (segfault, not JS exception)
Comment 1 Paolo Borelli 2013-03-12 23:12:26 UTC
Created attachment 238746 [details] [review]
Factor out a WorldView class
Comment 2 Giovanni Campagna 2013-03-13 17:49:30 UTC
I don't like hiding the model inside the view. It breaks the mvc split.

If you want to have a custom class that inherits from Gd.MainView, that's fine, but the model should be created by the window (or even better, by the application) and passed down.

Also, I'd like to investigate the gjs segfault.
Comment 3 Giovanni Campagna 2013-03-13 18:17:23 UTC
Ok, found the bug: GdMainView:model is a construct property, but GdMainView:view-type is not - yet, setting model relies on setting view-type to build the inner view.
Comment 4 Paolo Borelli 2013-03-13 18:33:25 UTC
(In reply to comment #2)
> I don't like hiding the model inside the view. It breaks the mvc split.
> 

That's why I think we should use composition instead of inheritance: to not call it "view" lets call it "WorldPanel" the world panel would be a widget that contains a View and the owns model. The Window has no business knowing the model and the view of each page, it is an high level object that contains the world panel and the city panel.

A refactoring to use EggFlowBox would be just an internal implementation detail of the world panel with no impact on the window
Comment 5 Paolo Borelli 2013-03-13 18:49:01 UTC
(In reply to comment #3)
> Ok, found the bug: GdMainView:model is a construct property, but
> GdMainView:view-type is not - yet, setting model relies on setting view-type to
> build the inner view.

Cool thanks for investigating, so a bug in libgd? should gjs still be able to survive?
Comment 6 Giovanni Campagna 2013-03-13 18:51:02 UTC
Yes, it's a bug in libgd. And funnily, I had found it two weeks ago, reported as bug 694533, and then completely forgot of it.
Comment 7 Giovanni Campagna 2013-03-20 18:47:45 UTC
Ok, so, I decided to experiment myself with eggflowbox. The end result does not look good, but I'm posting it anyway, at least it's feedback.

If you want to try it, you need to patch egg-list-box with bug 681224, otherwise the build system is unable to pick it.
Comment 8 Giovanni Campagna 2013-03-20 18:48:15 UTC
Created attachment 239403 [details] [review]
Add egg-list-box

For the main view we want to use a flow box, which is provided by EggListBox
until merged into Gtk.
Comment 9 Giovanni Campagna 2013-03-20 18:49:46 UTC
Created attachment 239404 [details] [review]
[WIP] Convert from GdMainView to EggFlowBox

EggFlowBox frees us from GtkTreeModel, so we can implement our data structures
directly in JS. It also frees us from GtkCellRenderer, allowing custom
complex widgets.
The disadvantage is that the selection model is that of a GtkIconView,
so we (temporarily) lose the separate selection mode and the check boxes
to indicate selection.
Comment 10 Paolo Borelli 2013-03-20 19:34:46 UTC
My (untested) idea for the selection was to turn off the builtin selection stuff and use GtkOverlay to put a checkbox over each widget
Comment 11 Giovanni Campagna 2013-03-26 14:58:33 UTC
Comment on attachment 238746 [details] [review]
Factor out a WorldView class

Attachment 238746 [details] pushed as 2357985 - Factor out a WorldView class
Comment 12 Giovanni Campagna 2013-06-08 17:20:51 UTC
After doing some more work on this, I pushed wip/egg-flow-box, with details of what's missing.

We should try to get those changes integrated in egg-flow-box (maybe as a egg-content-view?) before making the switch.
Comment 13 Giovanni Campagna 2014-08-27 14:23:14 UTC
Not interesting any more.