Skip to content

Conversation

pzrq
Copy link
Contributor

@pzrq pzrq commented Aug 9, 2017

macOS - git / local build

Before

before - body has a default margin
middle - still a scrollbar present

After

after - no scrollbars

Ubuntu VM - git / local build

Before

screen shot 2017-08-09 at 1 52 11 pm

After

screen shot 2017-08-09 at 1 53 37 pm

Windows VM - Via Evergreen patch build

https://evergreen.mongodb.com/version/598a8b6ae3c331594401b427

After

windows 10

width: 100%;
padding-left: 6px;
left: 50%;
margin-left: -26px;
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that the width of the loading icon is 26px hence the negative margin offset here?

This should probably be at the very least a locally scoped variable for verbosity or at least have an inline comment as it is not immediately obvious.

I would also consider using display: flex, height: 100vh, flex-direction: column on the parent with flex: 1 and width: 100% on the child as an alternative, cleaner approach.

@pzrq pzrq force-pushed the COMPASS-1417-scrollbars branch 2 times, most recently from 7ce6733 to 96d8e1e Compare August 10, 2017 03:35
@pzrq pzrq requested a review from matt-d-rat August 10, 2017 04:06
@pzrq pzrq force-pushed the COMPASS-1417-scrollbars branch from 96d8e1e to 64e2808 Compare August 10, 2017 04:07
@pzrq
Copy link
Contributor Author

pzrq commented Aug 10, 2017

CI is currently blocked on #1201

Copy link
Contributor

@matt-d-rat matt-d-rat left a comment

Choose a reason for hiding this comment

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

LGTM

@pzrq
Copy link
Contributor Author

pzrq commented Aug 10, 2017

Thanks @matt-d-rat 👍

@pzrq pzrq force-pushed the COMPASS-1417-scrollbars branch from 64e2808 to dbc568d Compare August 11, 2017 01:55
@pzrq pzrq merged commit 001c423 into master Aug 11, 2017
@pzrq pzrq deleted the COMPASS-1417-scrollbars branch August 11, 2017 02:15
pzrq added a commit that referenced this pull request Aug 11, 2017
* Temporarily disable hiding the loading animation

So it's easier to inspect its state.

* Remove default 8px margin

* Use standard top/left pattern from .sq CSS class

* Go back to width: 100% removing margin/padding

No flexbox needed in this context.

* Remove duplicate text-align

* Abstract towards a reusable loading animation component

CSS animation appears to be special enough I don't know how to handle it in this context, perhaps it needs another wrapper?

* Revert "Temporarily disable hiding the loading animation"
pzrq added a commit that referenced this pull request Aug 11, 2017
* Temporarily disable hiding the loading animation

So it's easier to inspect its state.

* Remove default 8px margin

* Use standard top/left pattern from .sq CSS class

* Go back to width: 100% removing margin/padding

No flexbox needed in this context.

* Remove duplicate text-align

* Abstract towards a reusable loading animation component

CSS animation appears to be special enough I don't know how to handle it in this context, perhaps it needs another wrapper?

* Revert "Temporarily disable hiding the loading animation"
pzrq added a commit that referenced this pull request Aug 11, 2017
* Temporarily disable hiding the loading animation

So it's easier to inspect its state.

* Remove default 8px margin

* Use standard top/left pattern from .sq CSS class

* Go back to width: 100% removing margin/padding

No flexbox needed in this context.

* Remove duplicate text-align

* Abstract towards a reusable loading animation component

CSS animation appears to be special enough I don't know how to handle it in this context, perhaps it needs another wrapper?

* Revert "Temporarily disable hiding the loading animation"
pzrq added a commit that referenced this pull request Aug 11, 2017
… (#1205)

* Temporarily disable hiding the loading animation

So it's easier to inspect its state.

* Remove default 8px margin

* Use standard top/left pattern from .sq CSS class

* Go back to width: 100% removing margin/padding

No flexbox needed in this context.

* Remove duplicate text-align

* Abstract towards a reusable loading animation component

CSS animation appears to be special enough I don't know how to handle it in this context, perhaps it needs another wrapper?

* Revert "Temporarily disable hiding the loading animation"
pzrq added a commit that referenced this pull request Aug 11, 2017
… (#1204)

* Temporarily disable hiding the loading animation

So it's easier to inspect its state.

* Remove default 8px margin

* Use standard top/left pattern from .sq CSS class

* Go back to width: 100% removing margin/padding

No flexbox needed in this context.

* Remove duplicate text-align

* Abstract towards a reusable loading animation component

CSS animation appears to be special enough I don't know how to handle it in this context, perhaps it needs another wrapper?

* Revert "Temporarily disable hiding the loading animation"
durran pushed a commit that referenced this pull request Aug 13, 2017
* Temporarily disable hiding the loading animation

So it's easier to inspect its state.

* Remove default 8px margin

* Use standard top/left pattern from .sq CSS class

* Go back to width: 100% removing margin/padding

No flexbox needed in this context.

* Remove duplicate text-align

* Abstract towards a reusable loading animation component

CSS animation appears to be special enough I don't know how to handle it in this context, perhaps it needs another wrapper?

* Revert "Temporarily disable hiding the loading animation"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants