Merge lp:~gangsterveggies/granite/fix-bug-1184202 into lp:~elementary-pantheon/granite/granite

Proposed by Pedro Paredes
Status: Merged
Approved by: David Gomes
Approved revision: 611
Merged at revision: 608
Proposed branch: lp:~gangsterveggies/granite/fix-bug-1184202
Merge into: lp:~elementary-pantheon/granite/granite
Diff against target: 29 lines (+19/-0)
1 file modified
lib/Widgets/DynamicNotebook.vala (+19/-0)
To merge this branch: bzr merge lp:~gangsterveggies/granite/fix-bug-1184202
Reviewer Review Type Date Requested Status
David Gomes (community) Approve
Review via email: mp+180757@code.launchpad.net

Commit message

DynamicNotebook: Fixed the bug #1184202 and the add_button's visibility is now optional.

Description of the change

Fixes the bug #1184202.

As requested, added a function set_button_visible (bool visible) that toggles the "New Tab" '+' button on and off depending on the 'visible' variable. Default setting is on.

To post a comment you must log in.
Revision history for this message
David Gomes (davidgomes) wrote :

If a property is called "add_button_visible", you should use Vala's automatic set/get and the functions would automatically be called "set_add_button_visible" and "get_add_button_visible", you can see many examples of this feature on DynamicNotebook.vala, here's one:

        /**
         * Allow duplicating tabs
         */
        bool _allow_duplication = false;
        public bool allow_duplication {
            get { return _allow_duplication; }
            set {
                _allow_duplication = value;

                foreach (var tab in tabs) {
                    tab.duplicate_m.visible = value;
                }
            }
        }

review: Needs Fixing
Revision history for this message
Pedro Paredes (gangsterveggies) wrote :

> If a property is called "add_button_visible", you should use Vala's automatic
> set/get and the functions would automatically be called
> "set_add_button_visible" and "get_add_button_visible", you can see many
> examples of this feature on DynamicNotebook.vala, here's one:
>
> /**
> * Allow duplicating tabs
> */
> bool _allow_duplication = false;
> public bool allow_duplication {
> get { return _allow_duplication; }
> set {
> _allow_duplication = value;
>
> foreach (var tab in tabs) {
> tab.duplicate_m.visible = value;
> }
> }
> }

I had done it differently to follow the bug description suggestion, but you are right. Fixed it in revision 609.

Revision history for this message
David Gomes (davidgomes) wrote :

Remove diff line 11.

review: Needs Fixing
Revision history for this message
David Gomes (davidgomes) wrote :

>private bool _add_button_visible = true;

bool _add_button_visible = true;

That's how all the other variables are declared above.

Revision history for this message
David Gomes (davidgomes) wrote :

Good job, thank you.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/Widgets/DynamicNotebook.vala'
--- lib/Widgets/DynamicNotebook.vala 2013-07-13 22:40:24 +0000
+++ lib/Widgets/DynamicNotebook.vala 2013-08-19 14:40:50 +0000
@@ -391,6 +391,25 @@
391 }391 }
392 }392 }
393393
394 /**
395 * Controls the '+' add button visibility
396 */
397 bool _add_button_visible = true;
398 public bool add_button_visible {
399 get { return _add_button_visible; }
400 set {
401 if (value != _add_button_visible) {
402 if (_add_button_visible) {
403 this.notebook.set_action_widget (null, Gtk.PackType.START);
404 } else {
405 this.notebook.set_action_widget (add_button, Gtk.PackType.START);
406 }
407
408 _add_button_visible = value;
409 }
410 }
411 }
412
394 public Tab current {413 public Tab current {
395 get { return tabs.nth_data (notebook.get_current_page ()); }414 get { return tabs.nth_data (notebook.get_current_page ()); }
396 set { notebook.set_current_page (tabs.index (value)); }415 set { notebook.set_current_page (tabs.index (value)); }

Subscribers

People subscribed via source and target branches