See the screenshot:
Description
Description
Details
Details
Related Objects
Related Objects
- Mentioned In
- T135911: GSoC: Pywikibot-Thanks | Community Bonding Meeting 1 | 15th May
T135933: GSoC: Pywikibot-Thanks | Community Bonding Meeting 2 | 20th May
T130585: Pywikibot Support for Thanks (GSoC Proposal)
T129049: Pywikibot support for Thanks - Mentioned Here
- T159302: Rephrase confusing "thanks" message on diff page
T89572: In Thanks confirmation, "No" choice title text should not say "Send a thank you notification to this user"
Event Timeline
Comment Actions
@matmarex: Do you have any thoughts on how we could fix this without breaking it elsewhere? Maybe adding an extra parameter to $.confirmable() saying that the line should be breakable?
Comment Actions
I don't think it's possible to animate this the way we do if we allow line-breaking (it relies on display: inline-block). Although it should be possible to allow line-breaking after the animation is done.
Comment Actions
Well, I have managed to replicate the bug.
Here's the screenshot with display: inline-block: http://ctrlv.in/726549
And here's the screenshot without it: http://ctrlv.in/726550
The issue, indeed, seems to be with when display: inline-block is applied.
Now I am not too familiar with jquery, but know basic JS. Should I still go ahead with this task?
Comment Actions
Replicated this on my manual wiki installation - here's a paste.
The issue doesn't seem to be in core, rather in Thanks itself.
A proper combination of css styles for jquery.confirmable's wrapper, interface, text and mw-thank-link will probably do it.
So far, it's a bit skewed for me: another paste of progress so far.
Could someone tell me if I'm on the right track, please? @kaldari @Legoktm
Thanks. :)
Comment Actions
The issue doesn't seem to be in core, rather in Thanks itself.
I suspect that we wont know the answer to this question until you've found a solution which works with jquery.confirmable.
Have you looked for related bugs in jquery.confirmable ?
A proper combination of css styles for jquery.confirmable's wrapper, interface, text and mw-thank-link will probably do it.
Yes, this is an approach that should be explored. It would be great if you can find a tweak to the Thanks css and/or js which fixes this bug.
Comment Actions
Yes, I have, in fact.
T89572: In Thanks confirmation, "No" choice title text should not say "Send a thank you notification to this user" seemed to be a good place to understand the jquery.confirmable code.
A proper combination of css styles for jquery.confirmable's wrapper, interface, text and mw-thank-link will probably do it.
Yes, this is an approach that should be explored. It would be great if you can find a tweak to the Thanks css and/or js which fixes this bug.
I'll go ahead with this, then! :)
Comment Actions
After looking at it in more detail and talking to @jayvdb , I think this is probably how I should approach this problem. Hardcoding changes in my console worked in the browser, but there are many issues that come along with that apporach, and making changes to Thanks directly was probably the wrong way of looking at things.
Working on this now.
Comment Actions
Change 283749 had a related patch set uploaded (by Darthbhyrava):
Non-breaking spaces break page layout when confirming thanks in diff view
Comment Actions
Added a timeout function after the transitions which allows for line-breaking.
Feedback, please?
Comment Actions
In the wake of @matmarex's observation that error F3940024 crops up due to my patch, I looked into more details.
This is how the layout of the diff page works. The old edit is in the left column of a table inside a div, while the new edit is to the right.
<div id="mw-content-text">
<table class=""diff diff-contentalign-left" data-mw="interface">
<colgroup></colgroup>
<body>
<tr style="vertical-align: top;">
<td class= "diff otitle"></td>
<td class="diff ntitle">
{content}
</td>
</tr>
</body>
</table>
</div>Inside the right column "diff ntitle", there are further divs titled "mw-diff-ntitlei", where i ranges from 1 through 5.
The error arises because of the div "mw-diff-ntitle1", where the height seems to be garbled once the 'No' button is clicked.
Also, changing the word-wrap property of the mw-diff-ntitle1 div decreases the height.
However, I can't seem to figure out why this div would be affected by the patch I submitted which changes only white-line and width properties of the interface div post transition.
Comment Actions
Besides, it would be nice if brackets would be kept with first and last words, in order to respect typographic rules (at least French typographic rules).
Comment Actions
Some things I've noticed that might help to fix this:
On .jquery-confirmable-interface, there is a
style="width: 0px;"
coming from somewhere. Before clicking cancel the value there is 346.312px, which looks like a calculation.
So I've searched MW for
style="width:
but did not find anything that seemed relevant. So I think this values is inserted during build by an automatic process.
I don't know how these processes work, so for now I just tried hardcoding the value so it doesn't change:
.jquery-confirmable-interface {
width: 346.312px !important;
}This way, clicking Cancel doesn't make the component drop to the bottom of the page, but the outcome is still not okay. The Cancel and Thank buttons are not removed.
When I change the value to something greater than 0, the text starts to get pulled back up.
At 200px it begins to looks almost normal (remember, it should be 346.312px). But we now have that 'thank' link there.
This screencap is from before clicking Cancel (i.e., before the action that changes the width value from 346.312 to 0):
Comment Actions
This is coming from the animation. Without your patch, there is an "opening" animation going on when the "Are you sure..." is appearing. In order to make it work, it has to start with width:0 and end with its natural width. When the string is not wrapping, the span "just" grows into the full line -- but when we make the string wrap, the changing width actually changes how much the text wraps -- which changes the height too.
I think there are two options here:
- Cancel the wrapping ONLY for diff pages (leave history pages as-is)
- Cancel the animation
Comment Actions
Change 445426 had a related patch set uploaded (by Hagar Shilo; owner: Hagar Shilo):
[mediawiki/core@master] Disable animation on thanks confirmation to prevent page break
Comment Actions
Removing task assignee due to inactivity, as this open task has been assigned to the same person for more than two years (see the emails sent to the task assignee on Oct27 and Nov23). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome.
(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)
Comment Actions
Maybe the real problem here is just that the string in German is way too long.
@daniel - Would it make sense to change "Dankeschön für diese Bearbeitung senden (stets öffentlich)?" to just "Senden Danke?" or "Senden Sie Danke?" or "Senden Sie öffentlich Danke?"? The English string is "Publicly send thanks?". I don't know any German, so I'm just guessing based off Google Translate. Surely there must be a shorter equivalent though.
Comment Actions
Yea, that's not just long, it's also rather awkward :)
You really want "Dank senden", meaning "send thanks". There is several ways to fit "publicly" in there, but none of them very elegant. I'd probably go for "Dank senden (für andere sichtbar)?" meaning "send thanks (visible to others)". We can also go for "Öffentlich Dank senden?". "Danke öffentlich senden" makes it sound like there's a non-public option as well (as in "send thanks publically?").
EDIT: I just looked at the screenshot. In this context, it's also possible to use "to thanks" as the verb: "Öffentlich danken" meaning "Thank publicly". Though it's not sure that's better.
Comment Actions
I changed it to "Dankeschön senden (für andere sichtbar)?" which is 19 characters shorter.
Comment Actions
I'd say the underlying problem are design decisions or implementations which either ignore or make strange assumptions about human languages and/or character widths. See also https://www.w3.org/International/articles/article-text-size.en
Comment Actions
Interesting! According to that page, this message should be expected to expand an average of 160–180% when translated into European languages. In this case, however, it expanded 281%. The new translation is a 190% expansion, so should be more in line with reasonable assumptions.
Comment Actions
Change 283749 abandoned by Thiemo Kreuz (WMDE):
[mediawiki/core@master] Non-breaking spaces break page layout when confirming thanks in diff view
Reason:
5 years old. It's probably not helpful to keep this in our review queues. It can still be found from the Phabricator ticket if needed.
Comment Actions
Change 445426 abandoned by Bartosz Dziewoński:
[mediawiki/core@master] Disable animation on thanks confirmation to prevent page break
Reason: