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.

Sign up for GitHub

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

Jump to bottom

Several small-ish things coming from the IOI #483

Merged
merged 5 commits into from Nov 29, 2015
Merged

Several small-ish things coming from the IOI #483

merged 5 commits into from Nov 29, 2015

Conversation

stefano-maggiolo
Copy link
Member

Review on Reviewable

@stefano-maggiolo stefano-maggiolo force-pushed the smallioithings branch 2 times, most recently from a374282 to 9c4d6f2 Compare October 15, 2015 22:14
wil93
wil93 reviewed Oct 19, 2015
View reviewed changes
cmsranking/static/Overview.js
}
}
if (user["team"] != null && user["team"] != undefined) {
return user['team'] + '-' + out;
Copy link
Member

Choose a reason for hiding this comment

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

From here, the indentation is broken

Copy link
Member Author

Choose a reason for hiding this comment

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

I brought shame on my name (used tabs).

@stefano-maggiolo
Copy link
Member Author

Fixed indentation and rebased.

wil93 added a commit to algorithm-ninja/cms that referenced this pull request Oct 28, 2015
@wil93
Merge pull request cms-dev#483 from stefano-maggiolo/smallioithings
582af3a 
Several small-ish things coming from the IOI
@wil93
Copy link
Member

wil93 commented Oct 31, 2015

Reviewed 1 of 1 files at r1, 6 of 6 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, 2 of 2 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

@lucach
Copy link
Contributor

lucach commented Oct 31, 2015

Please note that 257f646 has a side effect. The browser now renders a second, ugly and unuseful vertical scrollbar as the SidePanel div is a bit bigger.

See as example the screenshot attached.
schermata del 2015-10-30 20-51-37

The simplest fix (but maybe not the best one) is to drop the padding-top property.

@wil93 wil93 mentioned this pull request Nov 1, 2015
Closed
@wil93
Copy link
Member

wil93 commented Nov 1, 2015

A more "final" fix to the double scrollbar glitch is probably to remove overflow-y: scroll from div#InnerFrame and adapt the upper bar and the left sidebar to use position: fixed. This way only the "big" scrollbar is retained, and the smaller one will go away.

UPD: it's also better in general because, if you want to use the screenshot --fullpage feature of Firefox, the current method (overflow-y: scroll) causes to screenshot only the visible part of the webpage.

@stefano-maggiolo
Copy link
Member Author

I think overflow-y: scroll is there so that when you open the user details you don't have to scroll up. Anyway, for the moment I increased the spacing leeway to remove the scrollbar.

Review status: 8 of 13 files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

@landscape-bot
Copy link

Code Health
Repository health increased by 0.15% when pulling a7ec7b2 on stefano-maggiolo:smallioithings into 1d8e7fb on cms-dev:master.

@wil93
Copy link
Member

wil93 commented Nov 17, 2015

Reviewed 5 of 5 files at r7.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.04% when pulling f5d4a9f on stefano-maggiolo:smallioithings into e3641f7 on cms-dev:master.

@giomasce
Copy link
Member

Reviewed 3 of 6 files at r2, 1 of 1 files at r5, 1 of 2 files at r6, 2 of 5 files at r7, 1 of 5 files at r8.
Review status: 10 of 12 files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

@stefano-maggiolo stefano-maggiolo merged commit ceafd0e into cms-dev:master Nov 29, 2015
@stefano-maggiolo stefano-maggiolo deleted the smallioithings branch November 29, 2015 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@stefano-maggiolo @wil93 @lucach @landscape-bot @giomasce @artikz