Skip to content

Add NavDropdown + LinkContainer and MenuItem to visual-test #127

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.

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

Closed

Conversation

TimDaub
Copy link
Contributor

@TimDaub TimDaub commented Oct 16, 2015

Based on comment in #126

@taion
Copy link
Member

taion commented Oct 16, 2015

Sure. So

  1. We should add the NavDropdown to both examples.
  2. I don't think LinkContainer actually works with dropdowns right now

@taion
Copy link
Member

taion commented Oct 16, 2015

Should work after #112 and we can bring back the dropdown visual test.

But anyway if you want this merged, please do add the dropdown to the baseline example.

@TimDaub
Copy link
Contributor Author

TimDaub commented Oct 17, 2015

Ok,

will do it on monday.

On Sat, Oct 17, 2015, 01:41 Jimmy Jia [email protected] wrote:

Should work after #112
#112
and we can bring back the dropdown visual test.

But anyway if you want this merged, please do add the dropdown to the
baseline example.


Reply to this email directly or view it on GitHub
#127 (comment)
.

@taion
Copy link
Member

taion commented Oct 18, 2015

Might be superseded by #129 which adds back the old MenuItem thing.

@AlexKVal
Copy link
Member

I've rebased it on master with #128 and #129 merged and npm run visual-test
It looks as:
screen shot 2015-10-19 at 1 36 56 pm

Everything else seems OK too.

@taion
Copy link
Member

taion commented Oct 19, 2015

We should add the NavDropdown to both examples.

The point of this is that they match. Probably also add a title to the dropdown.

@taion
Copy link
Member

taion commented Dec 15, 2015

Closing for staleness.

@taion taion closed this Dec 15, 2015
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