Skip to content

Conversation

rerngvit
Copy link
Contributor

Add quickstart menu to the navigation bar for Flink documentation

@chiwanpark
Copy link
Member

Hi @rerngvit, Thanks for sending pull request. But your pull request has some problems.

First, {{ quickstart }} is not defined in your changes. As you can see in 19-22 lines of navbar.html, you should define {{ quickstart }}.

Second, there is no conditional active class for quickstart dropdown menu. Please include conditional active class in quickstart dropdown menu. Conditional active classes for other menus are in line 55 of navbar.html.

1. Define {{quick_start}}
2. Define active class for {{quick_start}} Dropdown menu
@rerngvit
Copy link
Contributor Author

@chiwanpark Thanks for reviewing this issue. I modified the pull request according to your comments. Please have a look.

@chiwanpark
Copy link
Member

Hi @rerngvit, I have tested your pull request.

Because setup_quickstart.html contains "setup" keyword in url, there are two active menus (Quickstart, Setup) when we open Quickstart: Setup page. I found a solution like following:

<li class="dropdown{% if page.url contains '/quickstart/' %} active{% endif %}">

The solution must be applied not only quickstart but also other dropdown menus such as setup, programming guide, ..., etc..

BTW, there is a typo line 68 of navbar.html. The a tag must be closed. I know this is not related to your changes but fixing it would be better.

After addressing this, we can merge this pull request. :-)

1. Add '/{link_name}/' for all dropDown toggle condition checks
2. Fix closing tag in line 68 (JobManager High Availability)
@rerngvit
Copy link
Contributor Author

@chiwanpark Thanks for reviewing this issue. I revised the pull request according to your comments. Please have a look.

@chiwanpark
Copy link
Member

Looks good to merge. I'll merge this.

chiwanpark pushed a commit to chiwanpark/flink that referenced this pull request Sep 24, 2015
…mentation

  - Improve conditional "active" class for dropdown menu
  - Fix unclosed A tag

This closes apache#1176.
@asfgit asfgit closed this in 0b23b5e Sep 24, 2015
@mxm
Copy link
Contributor

mxm commented Sep 25, 2015

Looks good. Thanks for the contribution @rerngvit!

@rerngvit rerngvit deleted the FLINK-2751 branch September 28, 2015 18:45
nikste pushed a commit to nikste/flink that referenced this pull request Sep 29, 2015
…mentation

  - Improve conditional "active" class for dropdown menu
  - Fix unclosed A tag

This closes apache#1176.
lofifnc pushed a commit to lofifnc/flink that referenced this pull request Oct 8, 2015
…mentation

  - Improve conditional "active" class for dropdown menu
  - Fix unclosed A tag

This closes apache#1176.
cfmcgrady pushed a commit to cfmcgrady/flink that referenced this pull request Oct 23, 2015
…mentation

  - Improve conditional "active" class for dropdown menu
  - Fix unclosed A tag

This closes apache#1176.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants