Page MenuHomePhabricator
Create Task
Maniphest T141286

Update site CSS customizations for the new change screen in Gerrit 2.12
Closed, ResolvedPublic
Actions

Description

In Firefox, the default font size used for commit messages and diffs is too small (10px). In Chromium, it is a larger, more reasonable size (12px).

This was fixed for the old change screen (see T46895 and T42941) by setting an absolute font size of 9pt in site CSS (9pt / (72 pt/in) * (96 px/in) = 12px), though apparently not for the new change screen, which is now the only option.

This, as well as other CSS that only affected the old change screen (e.g. word wrapping of the commit message), should either be changed to apply to the new change screen or removed from GerritSite.css.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
gerrit: Fix CSS selector for diff font size overrideoperations/puppetproduction+4 -1
Gerrit: fix gerrit's css class .commentPanelMessage nameoperations/puppetproduction+6 -4
Update gerrit css to use the new defined css in gerrit 2.12operations/puppetproduction+3 -4
Customize query in gerrit

Related Objects

Event Timeline

PleaseStand created this task.Jul 25 2016, 6:15 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 25 2016, 6:15 PM
Paladox subscribed.Jul 25 2016, 9:06 PM
gerritbot subscribed.Jul 25 2016, 9:45 PM

Change 301001 had a related patch set (by Paladox) published:
Update gerrit css to use the new defined css in gerrit 2.12

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

Paladox mentioned this in rOPUPd4484cc956e9: Update gerrit css to use the new defined css in gerrit 2.12.Jul 25 2016, 9:50 PM
Paladox mentioned this in rOPUP2bd364ab62a3: Update gerrit css to use the new defined css in gerrit 2.12.
Paladox mentioned this in rOPUP6ecd8189cf14: Update gerrit css to use the new defined css in gerrit 2.12.Jul 26 2016, 12:40 AM
Paladox mentioned this in rOPUPbcb2cdaceb57: Update gerrit css to use the new defined css in gerrit 2.12.Jul 26 2016, 12:51 AM
Paladox mentioned this in rOPUPd3a73e67e208: Update gerrit css to use the new defined css in gerrit 2.12.Jul 26 2016, 3:13 PM
Paladox mentioned this in rOPUPaf40b9de485d: Update gerrit css to use the new defined css in gerrit 2.12.
Dzahn mentioned this in rOPUP4d8ca0aee9bf: Update gerrit css to use the new defined css in gerrit 2.12.Jul 26 2016, 5:16 PM
gerritbot added a comment.Jul 26 2016, 5:18 PM

Change 301001 merged by Dzahn:
Update gerrit css to use the new defined css in gerrit 2.12

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

Paladox mentioned this in rOPUP2866283f5d15: Update gerrit css to use the new defined css in gerrit 2.12.Jul 26 2016, 5:21 PM
gerritbot added a comment.Jul 26 2016, 5:39 PM

Change 301172 had a related patch set uploaded (by Paladox):
Fix gerrit's css class .commentPanelMessage

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

Paladox mentioned this in rOPUP3ad8da71030f: Gerrit: fix gerrit's css class .commentPanelMessage.Jul 26 2016, 5:42 PM
Paladox mentioned this in rOPUPdf8646d04f78: Fix gerrit's css class .commentPanelMessage.
Paladox mentioned this in rOPUP47afd167968d: Gerrit: fix gerrit's css class .commentPanelMessage name.Jul 26 2016, 5:48 PM
Dzahn mentioned this in rOPUP06c246551ce2: Gerrit: fix gerrit's css class .commentPanelMessage name.Jul 26 2016, 6:34 PM
gerritbot added a comment.Jul 26 2016, 6:38 PM

Change 301172 merged by Dzahn:
Gerrit: fix gerrit's css class .commentPanelMessage name

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

Dzahn mentioned this in rOPUP94d72096be2b: Gerrit: fix gerrit's css class .commentPanelMessage name.Jul 26 2016, 6:39 PM
Paladox added a comment.Jul 27 2016, 10:40 PM

Hi ive uploaded two commits which should fix your issue, are you still having the same problem?

PleaseStand added a comment.Jul 28 2016, 7:29 AM
In T141286#2500516, @Paladox wrote:

Hi ive uploaded two commits which should fix your issue, are you still having the same problem?

The font size in the commit message box looks fine, but diffs look about the same. This seems to be because of a CSS rule that applies to table td and sets the font-size to small, reducing the font-size of 9pt you set on the table element.

One way to fix this problem would be to change your selector to .com-google-gerrit-client-diff-DiffTable_BinderImpl_GenCss_style-table .CodeMirror. Note the addition of .CodeMirror.

Paladox added a comment.Jul 29 2016, 10:59 PM

@PleaseStand hi but we do

.com-google-gerrit-client-diff-DiffTable_BinderImpl_GenCss_style-difftable .CodeMirror pre span {
white-space: pre-wrap !important;
}

with

.CodeMirror

Would you show me what css to use please from the above.

Paladox added a comment.Aug 4 2016, 1:14 PM

@PleaseStand do you have any suggestions please?

Paladox added a comment.Sep 6 2016, 3:11 PM

@PleaseStand bump, any updates on this please?

Aklapper triaged this task as Low priority.Sep 6 2016, 3:14 PM

@Paladox: No suggestions and no updates (assuming that answers the question, on behalf of PleaseStand).

Paladox added a comment.Sep 6 2016, 3:15 PM

I guess we can leave this open for another month and if no more reply's then close as resolve.

PleaseStand added a comment.Oct 5 2016, 4:26 AM
In T141286#2507172, @Paladox wrote:

@PleaseStand hi but we do

.com-google-gerrit-client-diff-DiffTable_BinderImpl_GenCss_style-difftable .CodeMirror pre span {
white-space: pre-wrap !important;
}

with

.CodeMirror

Would you show me what css to use please from the above.

I don't see that exact selector in GerritSite.css, so it probably comes from Gerrit's standard CSS. In any case, the selector used to specify white-space is different.

I believe I meant you should change .com-google-gerrit-client-diff-DiffTable_BinderImpl_GenCss_style-table to .com-google-gerrit-client-diff-DiffTable_BinderImpl_GenCss_style-table .CodeMirror. I did test this using Firefox Developer Tools, though not by editing GerritSite.css. (I do not have Gerrit installed locally.)

If you add pre span to the end of the selector, as you mentioned in your comment, the font size for the "... skipped n common lines ..." links is not adjusted. So don't include that.

If you have other questions for me, you can ask them, though it may be a while before I can try to answer them. A storm will affect my area within the next few days, and of course it's possible that the power outages will last for weeks.

PleaseStand added a comment.Oct 12 2016, 2:33 PM
In T141286#2691639, @PleaseStand wrote:

I believe I meant you should change .com-google-gerrit-client-diff-DiffTable_BinderImpl_GenCss_style-table to .com-google-gerrit-client-diff-DiffTable_BinderImpl_GenCss_style-table .CodeMirror. I did test this using Firefox Developer Tools, though not by editing GerritSite.css. (I do not have Gerrit installed locally.)

Fortunately, the storm caused very little damage in my area, and I did not lose power or Internet access, not even for a minute. I will post this change on Gerrit now.

gerritbot added a comment.Oct 12 2016, 2:34 PM

Change 315511 had a related patch set uploaded (by PleaseStand):
gerrit: Fix CSS selector for diff font size override

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

gerritbot added a project: Patch-For-Review.Oct 12 2016, 2:34 PM
gerritbot added a comment.Oct 13 2016, 9:59 PM

Change 315511 merged by Dzahn:
gerrit: Fix CSS selector for diff font size override

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

Dzahn subscribed.Oct 13 2016, 11:00 PM

applied on prod gerrit

PleaseStand closed this task as Resolved.Oct 26 2016, 7:36 AM
PleaseStand assigned this task to Paladox.

Marking this resolved because I can't think of any other customizations that would have to be fixed.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits