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

"TW" menu should come up as soon as possible #336

Closed
enterprisey opened this Issue Apr 19, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@enterprisey
Contributor

enterprisey commented Apr 19, 2016

The fact that the "TW" dropdown menu emerges a little bit later in the page loading process than everything else means that I misclick and edit pages instead of reading them (as the "Edit source" button moves under my mouse pointer, which used to be pointing at "Read") with depressing frequency.

Is it possible to make the "TW" dropdown menu appear faster in the loading script, and then have the rest of the menu loaded up a little later?

@quiddity-wp

This comment has been minimized.

Show comment
Hide comment
@quiddity-wp

quiddity-wp May 21, 2016

+1, this would be awesome to solve.

Maybe (IANAD!) we could use CSS to add a placeholder, before the async JavaScript adds the content/links?
(Just a thought, upon noticing my global.css seems to load near-instantly.)

quiddity-wp commented May 21, 2016

+1, this would be awesome to solve.

Maybe (IANAD!) we could use CSS to add a placeholder, before the async JavaScript adds the content/links?
(Just a thought, upon noticing my global.css seems to load near-instantly.)

@hartman

This comment has been minimized.

Show comment
Hide comment
@hartman

hartman Feb 16, 2017

Collaborator

Next week gadgets will be able to have peer modules.
https://www.mediawiki.org/wiki/ResourceLoader/Migration_guide_(users)#Gadget_peers

With this, it should be possible to fix this problem. You can have a hidden companion gadget, that is automatically enabled when you enable a primary gadget. This companion gadget can be a very simple styles only gadget, reserving space for the menu like:

.client-js.skin-vector #p-cactions {
    margin-right: 3em;
}

Then once the JS loads loads, add new CSS like:

.skin-vector #p-cactions + #p-twinkle {
    margin-right: initial;
}
.skin-vector #p-twinkle {
    width: 3em;
}

or something like that.

Collaborator

hartman commented Feb 16, 2017

Next week gadgets will be able to have peer modules.
https://www.mediawiki.org/wiki/ResourceLoader/Migration_guide_(users)#Gadget_peers

With this, it should be possible to fix this problem. You can have a hidden companion gadget, that is automatically enabled when you enable a primary gadget. This companion gadget can be a very simple styles only gadget, reserving space for the menu like:

.client-js.skin-vector #p-cactions {
    margin-right: 3em;
}

Then once the JS loads loads, add new CSS like:

.skin-vector #p-cactions + #p-twinkle {
    margin-right: initial;
}
.skin-vector #p-twinkle {
    width: 3em;
}

or something like that.

@MusikAnimal

This comment has been minimized.

Show comment
Hide comment
@MusikAnimal

MusikAnimal Sep 8, 2017

Collaborator

I gave this a go on testwiki and I think it worked! :)

@atlight Did you want to check it out? The new relevant files are MediaWiki:Gadget-Twinkle.css (main gadget) and MediaWiki:Gadget-Twinkle-pagestyles.css, loaded as a peer CSS-only gadget as hartman recommended.

I think going this route, it doesn't matter if the TW menu shows up or not (e.g. Special pages), because the main Twinkle.css is always loaded and will remove the CSS placeholder.

It looks like there's a conflict with MoreMenu however, where the CSS placeholder isn't properly removed. I think I can fix that, but also Twinkle is taking over the .client-js. skin-vector #p-cactions rule, so I can't do the same peer gadget trick with MoreMenu :(

Collaborator

MusikAnimal commented Sep 8, 2017

I gave this a go on testwiki and I think it worked! :)

@atlight Did you want to check it out? The new relevant files are MediaWiki:Gadget-Twinkle.css (main gadget) and MediaWiki:Gadget-Twinkle-pagestyles.css, loaded as a peer CSS-only gadget as hartman recommended.

I think going this route, it doesn't matter if the TW menu shows up or not (e.g. Special pages), because the main Twinkle.css is always loaded and will remove the CSS placeholder.

It looks like there's a conflict with MoreMenu however, where the CSS placeholder isn't properly removed. I think I can fix that, but also Twinkle is taking over the .client-js. skin-vector #p-cactions rule, so I can't do the same peer gadget trick with MoreMenu :(

@atlight

This comment has been minimized.

Show comment
Hide comment
@atlight

atlight Sep 9, 2017

Collaborator

It looks good on testwiki. I had a go at improving the selectors, although they might be too slow now - @hartman what do you think?

The issue of incompatibility with other gadgets is a problem though. You could always put a margin-left on the search portlet for MoreMenu, but then when a third gadget comes along that wants to do this, what will it do? Maybe we should leave that issue for future consideration and not net it hold us up for now.

It also occurred to me to check how will this work for people using enwiki in a RTL interface language, but it seems like the Twinkle menu already gets put in the wrong place in that case! We ought to fix that separately.

Collaborator

atlight commented Sep 9, 2017

It looks good on testwiki. I had a go at improving the selectors, although they might be too slow now - @hartman what do you think?

The issue of incompatibility with other gadgets is a problem though. You could always put a margin-left on the search portlet for MoreMenu, but then when a third gadget comes along that wants to do this, what will it do? Maybe we should leave that issue for future consideration and not net it hold us up for now.

It also occurred to me to check how will this work for people using enwiki in a RTL interface language, but it seems like the Twinkle menu already gets put in the wrong place in that case! We ought to fix that separately.

@MusikAnimal

This comment has been minimized.

Show comment
Hide comment
@MusikAnimal

MusikAnimal Sep 9, 2017

Collaborator

You could always put a margin-left on the search portlet for MoreMenu, but then when a third gadget comes along that wants to do this, what will it do? Maybe we should leave that issue for future consideration and not net it hold us up for now.

I agree. For MoreMenu I was going to use a pseudo-element to avoid conflicts with Twinkle, but obviously eventually gadgets will run out of selectors that they can use, and also won't know which ones are already being used. Hacky as can be but it's a good short-term solution, especially given Twinkle's popularity.

I was about to make a PR but I see you've got some new styles. They look good to me! What I said above about "doesn't matter if the TW menu shows up or not (e.g. Special pages)" is false. I did have to remove the CSS via JavaScript so that the timing was correct, but also I realize now we want TW on contributions pages :) I tested the appearance in a few browsers/OS's and all looked good.

Collaborator

MusikAnimal commented Sep 9, 2017

You could always put a margin-left on the search portlet for MoreMenu, but then when a third gadget comes along that wants to do this, what will it do? Maybe we should leave that issue for future consideration and not net it hold us up for now.

I agree. For MoreMenu I was going to use a pseudo-element to avoid conflicts with Twinkle, but obviously eventually gadgets will run out of selectors that they can use, and also won't know which ones are already being used. Hacky as can be but it's a good short-term solution, especially given Twinkle's popularity.

I was about to make a PR but I see you've got some new styles. They look good to me! What I said above about "doesn't matter if the TW menu shows up or not (e.g. Special pages)" is false. I did have to remove the CSS via JavaScript so that the timing was correct, but also I realize now we want TW on contributions pages :) I tested the appearance in a few browsers/OS's and all looked good.

@MusikAnimal

This comment has been minimized.

Show comment
Hide comment
@MusikAnimal

MusikAnimal Sep 12, 2017

Collaborator

Done with 3783550. Big thanks to @hartman for the working solution!

Collaborator

MusikAnimal commented Sep 12, 2017

Done with 3783550. Big thanks to @hartman for the working solution!

@hartman

This comment has been minimized.

Show comment
Hide comment
@hartman

hartman Sep 12, 2017

Collaborator

I had totally forgotten about this ticket... Thx for working on it people. !!!

Collaborator

hartman commented Sep 12, 2017

I had totally forgotten about this ticket... Thx for working on it people. !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment