Merge the 'collapsiblenav' component of Vector extension into the Vector skin
Closed, ResolvedPublic

Description

(Split from bug 45051.)

Considered stable, enabled on WMF wikis by default, currently configurable
in preferences. I don't like it personally and I have it disabled in my
prefs; it should probably be merged, but I'd like to keep the possibility of
disabling it in prefs.


Version: 1.19
Severity: normal

bzimport set Reference to bz46512.
matmarex created this task.Via LegacyMar 24 2013, 9:36 PM
lfschenone added a comment.Via ConduitMar 30 2013, 5:17 PM

I volunteer to take care of this.

lfschenone added a comment.Via ConduitMay 1 2013, 10:13 PM

After some idle weeks I return to work, sorry for the delay.
I'm new to Git and Gerrit, I've been looking carefully at this issue but I'm not sure how to proceed.
The collapsibleNav feature of the Vector extension has elements spread all over the extension files, and I'm not sure how to bring them to the Vector skin without generating conflicts later on with the merging of the other features. For example, the option to disable the collapsibleNav feature is triggered by a hook, which is a method of the VectorHooks class. This method uses a property of the class which contains references to other features of the Vector extension, such as the footerCleanup feature. Should I just remove everything not related to collapsibleNav from the VectorHooks class, and push the result? If I do this, then when merging the footerCleanup changes, a conflict may arise.
Should I push the VectorHooks class more or less as it is, even if it contains references to other features?
What is the right way to proceed in these cases?
Any help is appreciated, thanks.

MZMcBride added a comment.Via ConduitAug 13 2013, 6:40 PM

(In reply to comment #0)

Considered stable, enabled on WMF wikis by default, currently configurable
in preferences. I don't like it personally and I have it disabled in my
prefs; it should probably be merged, but I'd like to keep the possibility of
disabling it in prefs.

I don't like the user preference and think it should be killed. On the other hand, I don't use Vector.

(In reply to comment #2)
Bartosz: any chance you could reply to comment 2?

matmarex added a comment.Via ConduitAug 13 2013, 7:03 PM

After some though, I wouldn't mind killing the pref entirely. (I actually reenabled it for myself some time ago.)

(In reply to comment #3)

(In reply to comment #2)
Bartosz: any chance you could reply to comment 2?

I think the best way to answer this is, instead of poiting out each caveat, to point to fixes to bug 46513 and bug 46514, as this is exactly the same thing as there.

Jdlrobson added a comment.Via ConduitSep 9 2013, 10:02 PM

I'll look into this. It will help with some changes I am currently work on in Vector.

gerritbot added a comment.Via ConduitSep 10 2013, 12:01 AM

Change 83590 had a related patch set uploaded by Jdlrobson:
Move collapsibleNav to core

https://gerrit.wikimedia.org/r/83590

Reedy added a comment.Via ConduitSep 18 2013, 1:32 AM

Stats for enwiki and dewiki:

mysql:wikiadmin@db60 [enwiki]> select * from user_properties WHERE up_property='vector-collapsiblenav';
......
1584 rows in set (0.99 sec)

mysql:wikiadmin@db60 [enwiki]> select max(user_id) from user;
+--------------+

max(user_id)

+--------------+

19746121

+--------------+
1 row in set (0.00 sec)

0.008%

mysql:wikiadmin@db74 [dewiki]> select * from user_properties WHERE up_property='vector-collapsiblenav';
......
380 rows in set (0.28 sec)

mysql:wikiadmin@db74 [dewiki]> select max(user_id) from user;
+--------------+

max(user_id)

+--------------+

1733493

+--------------+
1 row in set (0.00 sec)

0.02%

Looks pretty low usage to me...

gerritbot added a comment.Via ConduitSep 18 2013, 1:46 AM

Change 83591 had a related patch set uploaded by Krinkle:
Move code for navigation collapsing to core

https://gerrit.wikimedia.org/r/83591

Krinkle added a comment.Via ConduitSep 18 2013, 2:14 AM

Note that those 1584 enwiki accounts have both value='' and value=1. They aren' all opt-in or opt-out.

Krinkle added a comment.Via ConduitSep 18 2013, 2:31 AM

Did another query limiting to where up_value!=1 (as it is enabled by default) and LEFT(user_touched, 8) >= '20130301' (filter out users who have no activity in the last 6 months).

Also ran it on an RTL wiki and multi-language wikis as they may be other trends there.

SELECT user_touched, up_property from user, user_properties WHERE up_user=user.user_id AND up_property='vector-collapsiblenav' AND up_value!=1 AND LEFT(user_touched, 8) >= '20130301';
...

  1. English, German, Dutch [enwiki] 902 rows in set (0.03 sec) [dewiki] 318 rows in set (0.39 sec) [nlwiki] 58 rows in set (0.36 sec)
  2. Hebrew [hewiki] 19 rows in set (0.05 sec)
  3. Multi-language [commonswiki] 159 rows in set (0.64 sec) [metawiki] 67 rows in set (0.34 sec)
gerritbot added a comment.Via ConduitOct 1 2013, 9:47 PM

Change 83590 merged by jenkins-bot:
Remove collapsibleNav module

https://gerrit.wikimedia.org/r/83590

gerritbot added a comment.Via ConduitOct 1 2013, 10:02 PM

Change 83591 merged by jenkins-bot:
Vector: Add navigation collapsing feature

https://gerrit.wikimedia.org/r/83591

Patrick87 added a comment.Via ConduitNov 5 2013, 12:12 AM

Excerpt from bug 55905, comment 10:

  1. The new implementation of the collapsiblenav seems to be imperfect: On initial page load the toolbox is not collapsible and all items are expanded. After some delay the collapsiblenav content seems to be loaded (e.g. arrows appear, indentation changes) and some of the items collapse. It should be collapsible from the start or not collapsible at all - not something in between that gets slowly loaded with notable delay after page load.

I think the implementation should be optimized especially after the option to disable the collapsing was killed.

Add Comment