Skip to content
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

[docs] Add API menu and demo button #6455

Merged
merged 2 commits into from Mar 29, 2017
Merged

[docs] Add API menu and demo button #6455

merged 2 commits into from Mar 29, 2017

Conversation

oliviertassinari
Copy link
Member

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation next labels Mar 28, 2017
@muibot muibot added PR: Needs Review PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels Mar 28, 2017
@oliviertassinari oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Mar 28, 2017
@mbrookes
Copy link
Member

mbrookes commented Mar 29, 2017

Clean-up is pretty sweet. Doubt I could ever have got it looking that good, never mind so quickly, but:

  • Menus don't work for pages outside of component-demos (Icons, Typography, Themes etc). Deleting is cheating! 😄

  • Menus don't work between API pages.

  • Menu appearance is weird:

Can't see it there, but as well as the odd highlighting, links are underlined. (component={Link})

  • Some nitpicky stuff on file-naming and placement.

@oliviertassinari
Copy link
Member Author

Menus don't work for pages outside of component-demos

We need to change the tree structure to make it work. It needs to carry more information. Still, answering 80% of the issue. That was simpler to begin with.

Menus don't work between API pages

That was simpler to implement without that feature. I don't think that it's the core of the issue, hence put it aside.

Menu appearance is weird

Yep, that's an issue at the menu level, I think that we should able to use simple link over imperative API to navigate.

Thanks for the review.

@mbrookes
Copy link
Member

We need to change the tree structure to make it work.

I had already changed the tree structure, and this was already working. (Ignoring that I had barely got started on the rewrite when I stopped to get feedback on the new tree structure).

That was simpler to implement without that feature.

I can take care of it.

I don't think that it's the core of the issue

It most definitely is.

master has all the related APIs on one page, next doesn't. To get from one to the other on next is painful, especially at a smaller window sizes where the side-nav closes when you select a page. ('smaller', i.e. 1280dp, being nearly full screen on a 15" retina MBP).

Since you can't view them together, we need to make getting from one to the other much easier.

@oliviertassinari
Copy link
Member Author

I had already changed the tree structure

Sorry, I'm not referring to the file tree structure, but the data structure.

It most definitely is.

I agree, there is a need for it, I was think of having a AutoComplete search field to solve more complexe use cases like this one.

Do you think that we can merge that PR and continue the iteration?

@mbrookes
Copy link
Member

Sorry, I'm not referring to the file tree structure, but the data structure.

The flattened directory structure simplified populating the data structure.

Like I said, this was already working (just far from its final form).

I agree, there is a need for it, I was think of having a AutoComplete search field to solve more complex use cases like this one.

To use AutoComplete to find related components, you need to know what you're searching for. In most cases related components have the same base name, but not always. I'd see search / AutoComplete across the entire API as complementary to something that explicitly lists the related components.

Do you think that we can merge that PR and continue the iteration?

Sure. I'm probably going to tweak some of the file and function name changes, but that can be part of a separate PR that completes / reinstates the missing parts.

This is a step forward as is.

@mbrookes mbrookes merged commit 87ff7d1 into mui:next Mar 29, 2017
@oliviertassinari oliviertassinari deleted the docs-v1 branch March 29, 2017 12:09
@oliviertassinari
Copy link
Member Author

This is a step forward as is.

I agree with your points 🎉 , let's get a nice UX for the docs 😄 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants