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

[WIP] Docs redesign #7530

Merged
merged 84 commits into from Jan 25, 2019
Merged

[WIP] Docs redesign #7530

merged 84 commits into from Jan 25, 2019

Conversation

colleenmcginnis
Copy link
Contributor

This PR introduces largely cosmetic changes to the Mapbox GL JS docs. Changes include:

  • Making use of the shared /dr-ui React components, which introduces consistency between the layout in this documentation and other Mapbox docs sites.
  • Transitioning from base.css to assembly classes.
  • Adding an examples landing page (in the style of the Maps SDK for iOS and other examples pages).
  • Treating the Mapbox Style Specification as its own product in the product menu dropdown. (To do: make these changes in the /dr-ui repo.)

Off to @mollymerp for a preliminary review!

cc @mapbox/docs

@ryanhamley
Copy link
Contributor

I love this redesign! It's so bright and clean. Would it be possible to keep the search bar on the examples list? There's no rhyme or reason to how the examples are ordered so the search bar comes in handy quite often.

@colleenmcginnis
Copy link
Contributor Author

@ryanhamley yes! Thanks for flagging this. I actually added that existing search bar in my first few weeks at Mapbox 👶and I'm sure I could build something better now. This would also be useful for examples pages on other docs sites. I'll work on building a component we can use across repos.

Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

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

Thank youuuuu @colleenmcginnis this is awesome. I did a first pass here :)

A couple things I noticed:

  • the url hash locations (ex http://localhost:8080/mapbox-gl-js/api/#map#unproject) don't take the header into account, so the hashed item is covered by the header when you navigate to an item and it also does not expand the item
  • text wrapping isn't working correctly in the options table for long option names

image

  • page kinda jumps around when you expand/collapse API methods 🤔
  • this also happens in the old docs on Firefox, but it would be nice to fix if we can – the "Related" examples have weird alignment:

image

docs/pages/overview/roadmap.js Outdated Show resolved Hide resolved
docs/pages/plugins.js Show resolved Hide resolved
docs/pages/plugins.js Outdated Show resolved Hide resolved
docs/pages/plugins.js Outdated Show resolved Hide resolved
docs/components/quickstart.js Outdated Show resolved Hide resolved
docs/pages/overview/index.md Outdated Show resolved Hide resolved
docs/components/page_shell.js Outdated Show resolved Hide resolved
docs/components/page_shell.js Outdated Show resolved Hide resolved
docs/components/page_shell.js Outdated Show resolved Hide resolved
@mollymerp
Copy link
Contributor

also if you rebase this branch on master that should fix the failing unit tests.

@colleenmcginnis
Copy link
Contributor Author

@mollymerp this is ready for another look whenever you are!

Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

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

Nice work!! I'm noticing a couple funky things that may be related to the dr-ui components or batfisk or something else idk
when I click from the GL JS to the Style Spec, the drop-down thinks I've selected both:

image

its also weirdly slow when I click on a sidebar item (300ms on the GL JS API reference, and over 1 second on the style-spec page) – looks mostly related to setting state and parsing markdown. Is the whole style-spec being stored as state? I could imaging that diffing might take a long time 💭

image

I didn't have time to go through fully today, but thought I'd post my comments so far so I don't block ya. I'll come back to this tomorrow!

docs/components/api-item-member.js Show resolved Hide resolved
docs/components/api-item-member.js Outdated Show resolved Hide resolved
{section.license && <div>License: {section.license}</div>}
{section.author && <div>Author: {section.author}</div>}
{section.copyright && <div>Copyright: {section.copyright}</div>}
{section.since && <div>Since: {section.since}</div>}
Copy link
Contributor

Choose a reason for hiding this comment

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

what are these parts for? I'm not seeing them when I run the page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm I'm not sure either actually. This is currently included here https://github.com/mapbox/mapbox-gl-js/blob/mb-pages/docs/pages/api.js#L197-L202.

@colleenmcginnis
Copy link
Contributor Author

When I click from the GL JS to the Style Spec, the drop-down thinks I've selected both:

Ya I noticed that too. It will have to be addressed in /dr-ui. 😕

Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

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

seeing a couple small things on mobile:
text wrap is working for the option name column, but not the description column
img_77ff76c1eea1-1

lil floating white arrow over GL JS when opening the dropdown:
img_ed9e2c61fbe1-1

Otherwise this looks good to me! going to tag in one more reviewing from the GL team because this is such a large PR.

.map(({frontMatter}) => frontMatter);
.map(example => {
return {
path: example.path,
Copy link
Contributor

Choose a reason for hiding this comment

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

why does pathname not work for the path here (i.e. why do you need to transform the frontmatter)?

@katydecorah
Copy link
Contributor

katydecorah commented Jan 25, 2019

I added a unit test for the example pages to assert title, metadata, tags, and pathname https://github.com/mapbox/mapbox-gl-js/blob/4b8113e1f480f1ea6e1e0ad5b0c77373230041b9/test/unit/docs/examples.test.js

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.

None yet

4 participants