-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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] Implement the new API display design #37405
Conversation
Netlify deploy previewhttps://deploy-preview-37405--material-ui.netlify.app/ Bundle size report |
d9c3bf4
to
bbabcc1
Compare
@alexfauquette maybe it's ok to make just the anchor icon container be clickable? The trade-off seems reasonable here. We also end up winning less hover state to manage, I guess! |
For the |
@alexfauquette This last commit of mine, for some reason, broke the scroll anchoring 😅 Couldn't find where to fix it, so just giving you a shout out! |
@danilo-leal it's the deletion of I've continued the modification by
|
I moved type description from the body to the header of props API. To simplify the alignment I also reorganized the <div className="prop-list-additional-info">
<div className="prop-list-default-props">// This is a table row
<div className="prop-list-title">// This is a table cell
<p>Default:</p>
</div>
<div className="prop-list-content">// This is a table cell
<code>{propDefault}</code>
</div>
</div>
{/* Repeat for types and signature */}
</div> |
@alexfauquette this is amazing, thanks for working on this! I ended up customing the alignment a bit more, simplifying it to use Lastly, is there a scalable way ⎯ instead of going case by case ⎯ to remove the |
Thanks for the feedback item, it's much better 👌 I removed the For me we are good for merging 👍 |
Great work everyone! I really like this last iteration with the |
Interesting take! @alexfauquette had originally put it on the top of the page but I ended up thinking that moving it to the bottom would be better because if you're not in the mood to give feedback, it's extra space + scroll that we're requiring from you. Although the idea of having it on the top just for the first week isn't bad, either. But maybe we could call attention out to this one on Twitter/blog and expose that there's such a place to drop feedback? |
I had same concern as @zanivan when creating it. My thought process was the next one:
Having it at the bottom seems to be a nice compromise because we have components with very small API pages for which it will be easier to see the warning. |
@alexfauquette it looks great 👌 |
@alexfauquette I have noticed an important regression from these changes, the API pages are no longer indexed in Algolia. For example, "autoHighlight" does no longer return any results. To compare with https://v4.mui.com/. To fix this, we need to go to https://crawler.algolia.com/admin/crawlers/739c29c8-99ea-4945-bd27-17a1df391902/configuration/edit and update the crawler configuration. Could you fix this? I could fix it but I feel it would be a great opportunity to learn more about the crawler if you haven't already. Thanks! |
@alexfauquette Thanks, you fixed it :) lvl3: [".algolia-lvl3", ".markdown-body h3, .MuiApi-item-title"], |
@oliviertassinari Even better, now the autoHighligh search leads you to the props itself (not just to the props table 😍) |
Links
This PR is aimed at improving the API tables browsing experience as reported in the following issues:
Fix #34085.
Fix #36544
Design reference here.
Previews
To-do:
It's far from perfect in terms of dev and CSS but all the basics are here.