Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide space for portlet if unused #815

Closed
wants to merge 2 commits into from
Closed

Hide space for portlet if unused #815

wants to merge 2 commits into from

Conversation

DannyS712
Copy link
Contributor

Closes #809

@Amorymeltzer
Copy link
Collaborator

@DannyS712 I asked this in #809, but is there a way to get a visual, on-screen change?

@DannyS712
Copy link
Contributor Author

@DannyS712 I asked this in #809, but is there a way to get a visual, on-screen change?

What do you mean?

@Amorymeltzer Amorymeltzer changed the title Hide space for portel if unused Hide space for portlet if unused Jan 24, 2020
@Amorymeltzer
Copy link
Collaborator

Amorymeltzer commented Jan 24, 2020

I mean, it's clear that Twinkle is affecting the margin-right of Vector's p-cactions, but, as someone who doesn't regularly use Vector, what's a good way to visually notice this, like by having other gadgets enabled or something? Not, like, vital, but wondering for testing, etc.

twinkle.js Outdated Show resolved Hide resolved
Per discussion on PR

Co-Authored-By: Amory Meltzer <Amorymeltzer@gmail.com>
@siddharthvp
Copy link
Member

@DannyS712 I asked this in #809, but is there a way to get a visual, on-screen change?

Get yourself desysopped or use a non-admin account and go to PrefixIndex in vector skin. There's an empty space next to the "More" button. Or am I misunderstanding your question?

@siddharthvp
Copy link
Member

@Amorymeltzer raises a very valid point. Unsetting the CSS via JS causes the menu to jump, (which is ironically exactly what the css is supposed to prevent, though in the opposite way). We have to decide between which is a greater annoyance: an empty space where there should have the TW menu, or jumping back of the "More" to its original position?

At PrefixIndex, the jump isn't much of an issue as there's just the More button. But after #793, the issue could arise on other pages as well (Read, Edit, View history would all jump) for people who choose to disable several modules.

I wonder what @MusikAnimal thinks about this? He added the css originally.

@Amorymeltzer
Copy link
Collaborator

Yes that's the reason I ask. And yes, @siddharthvp, but I don't have any menu there in vector on my nonsysop account. What's in it? You two are obviously seeing something, but I've got a fairly clean/default gadget list for that account, so I'm assuming you both have put something in that menu that exposes the issue.

@siddharthvp
Copy link
Member

I don't have any menu there in vector on my nonsysop account.

Oh yeah, those items in "More" are from userscripts. You should be able to get one by running mw.util.addPortletLink('p-cactions', '#', 'BAH');, or by installing some random script.

@Amorymeltzer Amorymeltzer added bug Module: twinkle The twinkle.js global gadget file labels Jan 24, 2020
@Amorymeltzer
Copy link
Collaborator

As @siddharthvp noted and I suspected, this means a jump is back in play. I suppose it's better than an empty space?

@Amorymeltzer
Copy link
Collaborator

Circling back to this, what do folks think? I guess jumping is better than a lingering/ghost space?

@MusikAnimal
Copy link
Collaborator

In Twinkle, the only items that could show up at Special:PrefixIndex are for sysops, right? One possible solution would be to move the .client-js > body.skin-vector.mw-special-Prefixindex #p-cactions rule (and the other sysop-only rules, I guess) to a separate hidden peer gadget with rights=sysop in the definition.

At any rate, I can't imagine Special:PrefixIndex is super high-traffic, so it's not the end of the world to have empty space or jumping. If we have to chose between the two, I'd go with the latter. This is the way I always did it for MoreMenu, and I never received complaints. Remember, before the peer gadget stuff went live, everyone had jumping when the TW menu first loaded. So if 99% of the time you don't see jumping (the other 1% being at Special:PrefixIndex), that's still a win.

@Amorymeltzer
Copy link
Collaborator

Thanks MA — that's not a bad idea for the sysop-only bit, although as @Sd0001 points out it'll become more pronounced with #793. That roughly jives with what I figure, but then again I'm not on Vector so I really don't know how bad the jumping feels!

@Amorymeltzer Amorymeltzer added this to the March 2020 update milestone Feb 12, 2020
Amorymeltzer added a commit that referenced this pull request Feb 16, 2020
Closes #809

Update twinkle.js

Per discussion on PR

Co-Authored-By: Amory Meltzer <Amorymeltzer@gmail.com>
Copy link
Collaborator

@Amorymeltzer Amorymeltzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna edit to add a comment, but looks good!

@Amorymeltzer
Copy link
Collaborator

Done in 5cff2a8 (though I forgot to fix the typo)

wiki-ST47 pushed a commit to wiki-ST47/twinkle that referenced this pull request Sep 2, 2020
Closes wikimedia-gadgets#809

Update twinkle.js

Per discussion on PR

Co-Authored-By: Amory Meltzer <Amorymeltzer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Module: twinkle The twinkle.js global gadget file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Space for portlet added without portlet
4 participants