Skip to content

Migrate to bootstrap 4 - #1209

Merged
chalin merged 21 commits into
masterfrom
chalin-bootstrap-4-0829
Aug 31, 2018
Merged

chalin merged 21 commits into
masterfrom
chalin-bootstrap-4-0829

Conversation

@chalin

@chalin chalin commented Aug 30, 2018 •

Copy link
Copy Markdown
Contributor

@chalin chalin added the infra.structure Relates to the tools that create docs.flutter.dev label Aug 30, 2018
@chalin chalin added this to the 18Q3 milestone Aug 30, 2018
@chalin
chalin requested a review from Sfshaza August 30, 2018 20:03
@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Aug 30, 2018
@Sfshaza

Sfshaza commented Aug 30, 2018

Copy link
Copy Markdown
Contributor

Wow! What a lot of work, @chalin. I scanned the file changes and played with the staged site. It looks great!

@Sfshaza

Sfshaza commented Aug 30, 2018

Copy link
Copy Markdown
Contributor

LGTM from me, but waiting for @filiph to weigh in.

@filiph filiph left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really good stuff, thank you! I like that you added new documentation comments as well.

Added a few nits but feel free to land.

Comment thread src/_includes/head.html
Comment thread src/_includes/header.html
{% include main-nav.html class="nav-item" %}
</div>

<div class="site-nav-vertical dropdown">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[nit] The current version shows the drop down on mouse-over, and it also shows cursor: hand.

@chalin chalin Aug 31, 2018 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The hover behavior is non-standard for menus/dropdowns, and doesn't work on mobile. I'll address the cursor in a followup PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll address the cursor in a followup PR.

Fixed in #1220.

Comment thread src/_layouts/default.html
@chalin
chalin merged commit 8c5ef5e into master Aug 31, 2018
@chalin
chalin deleted the chalin-bootstrap-4-0829 branch August 31, 2018 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Contributor has signed the Contributor License Agreement infra.structure Relates to the tools that create docs.flutter.dev

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants