Page MenuHomePhabricator

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

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 Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 25 2016, 6:15 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

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

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

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

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

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

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

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

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.

@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.

@PleaseStand do you have any suggestions please?

@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).

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

@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.

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.

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

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

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

Dzahn added a subscriber: Dzahn.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.