twinkle.js: group all portlet code in a class #1836
Conversation
github-actions
bot
added
Module: tag
Module: speedy
Module: warn
Module: protect
Module: arv
Module: welcome
Module: xfd
Module: prod
Module: block
Module: batch
They are configurable by editing twinkleoptions.js directly (which is why they are in the
It's defined in vector 2010 skin, comes from |
|
My searches indicate that 6 users are using the hidden skin preferences, one of which is probably a developer's test account. Not ideal to disrupt user's preferences, but I think it's acceptable since this patch will enable further refactoring that will result in the portlet code being much more maintainable. |
|
This patch is kind of big, and I have some reservations about introducing ES6 classes when the rest of the code is still on ES5 style classes. I think I'll abandon this patch and then submit a smaller patch that just eliminates portlet preferences. |
- No longer allow users to set portlet preferences. These hidden preferences were rarely used (6 instances found), and increase technical debt by making the code complex. I think it makes sense to eliminate them. - Move defaults for these values to the Twinkle.addPortlet() function, a more natural spot for these. - One of these preferences was used outside the Twinkle.addPortlet() function, so modify Twinkle.addPortlet() to return the preference and place it in a variable. - Add a comment about what $.collapsibleTabs is. Related wikimedia-gadgets#1836 Note to self: not manually tested yet.
I think that's fine, as we have to start somewhere. Can't refactor the whole codebase to ES6 in one go. |
* no longer allow users to set portlet preferences - No longer allow users to set portlet preferences. These hidden preferences were rarely used (6 instances found), and increase technical debt by making the code complex. I think it makes sense to eliminate them. - Move defaults for these values to the Twinkle.addPortlet() function, a more natural spot for these. - One of these preferences was used outside the Twinkle.addPortlet() function, so modify Twinkle.addPortlet() to return the preference and place it in a variable. - Add a comment about what $.collapsibleTabs is. Related #1836 Note to self: not manually tested yet. * fix bug * improve docblocks * fix bugs * improve docblock * add comments * comments. remove "type" variable * remove unneeded values
Moved away from storing skin variables in the preferences system, which is confusing, makes them seem like they're configurable at Twinkle/Preferences, but they are not.
Also renamed a variable, and deleted some variables that weren't used.
Later, will refactor that class a bit more so that we can add some unit tests. For example,
documentshould probably be passed in through the constructor so it can be mocked.What is
$.collapsibleTabs? Can that be deleted? Doesn't look like it's ever set.