FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix horizontal margins on mobile by iLtc · Pull Request #32 · techcorridorio/techcorridorio.github.io · GitHub

Fix horizontal margins on mobile - #32

Merged
benjaminoakes merged 7 commits into
techcorridorio:masterfrom
iLtc:master
Feb 11, 2017
Merged

Fix horizontal margins on mobile#32
benjaminoakes merged 7 commits into
techcorridorio:masterfrom
iLtc:master

Conversation

iLtc commented Dec 1, 2016

Copy link
Copy Markdown
Contributor

I fixed the horizontal margins on mobile.

However, it causes another margin problem on the desktop version.

I can not find an elegant solution. I just override the css attribute.

It looks good now.

dahlbyk commented Jan 7, 2017

Copy link
Copy Markdown
Member

I believe the margins can be more simply fixed by adding .col-md-12 to .page-heading in its own row:

    <div class="row">
        <div class="page-header col-md-12">
            <h1>{{ page.header }}</h1>
        </div>
    </div>
    <div class="row">
        <div class="col-md-12">
            {{ content }}
        </div>
    </div>

iLtc commented Jan 8, 2017

Copy link
Copy Markdown
Contributor Author

Actually, the problem is that there is one .row inside another .row. This is not necessary. After checking the official example, which put .page-header out of the .row, I removed the outer .row.

If you want to keep the .page-header inside the .row, we can either write the two .row separately or remove the inner .row. And you are right, we need add .col-md-12 to .page-header.

Also, this will still cause the margin problem on the desktop version. And we still need to override the css attribute.

dahlbyk commented Jan 8, 2017

Copy link
Copy Markdown
Member

Yeah, I think I have it right with two consecutive rows, one with .page-heading and one for content. I tried the change in Chrome and Firefox dev tools and it seems to work as expected? Not sure why the CSS change would still be required… this is a pretty basic Bootstrap layout.

iLtc commented Jan 9, 2017

Copy link
Copy Markdown
Contributor Author

Ok, I fixed it by using the two consecutive rows. I also removed the CSS.

Please notice that, on the desktop version, there is a horizontal margin problem now. 😭

Copy link
Copy Markdown
Member

Please notice that, on the desktop version, there is a horizontal margin problem now. 😭

@iLtc Did you want to make any changes to address that?

This does seem better on mobile, so it could make sense to merge now.

iLtc commented Feb 4, 2017
edited
Loading

Copy link
Copy Markdown
Contributor Author

Since I can't solve that, we can merge now.

By the way, this is what I will do next~

Copy link
Copy Markdown
Member

Let's move the "next meetup" feature to another PR, since it's off-topic.

iLtc commented Feb 5, 2017

Copy link
Copy Markdown
Contributor Author

God, I don't know push to my repository will influence here.

Hopefully, it works now.

dahlbyk commented Feb 11, 2017

Copy link
Copy Markdown
Member

God, I don't know push to my repository will influence here.

Tip: never open PRs from master. 😀

benjaminoakes merged commit e15609a into techcorridorio:master Feb 11, 2017

Copy link
Copy Markdown
Member

Thanks @iLtc!

iLtc added a commit to iLtc/techcorridorio.github.io that referenced this pull request Feb 11, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL