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

Added docs for React Tips section Introduction and Inline Styles Rea... #362

Merged
merged 67 commits into from
Nov 14, 2013

Conversation

mcsheffrey
Copy link
Contributor

...ct entry.

@mcsheffrey mcsheffrey mentioned this pull request Sep 23, 2013
### Solution
Instead of writing a string, create an object whose key is the camelCased version of the style name, and whose value is the style's value, in string:

```html
Copy link
Member

Choose a reason for hiding this comment

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

js :)

@zpao
Copy link
Member

zpao commented Sep 25, 2013

I think it might be better to put these under /cookbooks instead of putting more under /docs. What do you think?

@mcsheffrey
Copy link
Contributor Author

Yeah, that was my thought too. I just didn't want to break up the structure too much since I'm new here. But, if you think so I'll go ahead and make that change. :)

@mcsheffrey
Copy link
Contributor Author

@zpao was discussing scaling the cookbook section with @chenglou. I think the current layout works for the first 10-20 cookbook entries.

He brought up it would be helpful as the section grows (50 entries+) to pull out the Jekyll yaml front matter block from the entries so we just have plain markdown files in the cookbook directory, and then have a grunt file that generates thefront matter block (title, permalink, js includes) for each page based on the filename.

Doesn't look like Grunt is currently a dependency for the docs section. Wondering if that's something we want to add in the future?

@zpao
Copy link
Member

zpao commented Sep 26, 2013

Hmm, I might have been sleep-talking, or maybe it wasn't me. Either way, let's tackle that problem if/when we get there and keep front matter in and not add any Grunt dependency.

@mcsheffrey
Copy link
Contributor Author

@zpao Ha, sorry, should've written "I was discussing scaling the cookbook section with @chenglou". No sleep-talking on your side. :) Tackling that problem if/when we get there sounds good to me. Thanks!

mcsheffrey and others added 23 commits October 29, 2013 10:14
…oop through cookbook yaml. Added cookbook directory to js/ to add live editing of code samples
Was html before bc github screws up the js highlighting for jsx.
- No need to mention React, you know you're working with it =).
- Wrap code elements in back ticks, so that they get the "box" styling for md.
- You'd want the snippet to work inside the live editor, so you need to `renderComponent`. As per wiki specification, the DOM element on which to mount is `mountNode`, just like on the front page.
- Don't forget the JSX pragma, or else your `render` fails.
- Nitpick: empty line after the end of md.
- No need for jQuery reference since
  1. The general mood around React is that you don't need jQuery.
  2. The syntax' still clear without jQuery.
  3. We're doing a jQuery integration entry =).
- `getInitialState` was absent.
- You don't need `componentWillMount` here. fetch them in `getInitialState`.
- The non-spoken convention seems to call the event handler `"handle" + eventName`. So `handleResize` clearly indicates it's a `resize` handler while `updateDimensions` might do something else. This latter name might actually be better under circumstances where you use call the method directly somewhere, but since we removed the only direct usage in `componentWillMount` this is fine.
- Went OCD again and tried to keep the code short. `width` is enough of a demo. Removed `height`.
- Distinguish between DOM events and React events. Wish we go full React events in a near future.
@zpao
Copy link
Member

zpao commented Nov 5, 2013

Ok, I've been slowly digesting this over the past few days and here are my thoughts (some we talked about):

  1. Let's rename this "Tips" or something. Since we deviated from the cookbooks format, it feels a bit disingenuous to call them that.
  2. We should make sure we keep filename, page title, and sidebar link title in sync, at least for this first push. I think they all started that way but with tweaking they've fallen out of sync. Also, make sure titles are title-cased (I noticed it mostly in the sidebar).
  3. Feeding off 2… if we go with the titles we have in the files, I think some will span multiple lines in the sidebar. We've avoided this so far because it's a pretty bad experience. I'll leave it up to you to make the decision on what happens here.
  4. The order of entries is seemingly random. Style is talked about in 2 docs but they're split by several other unrelated docs. Let's bring these together. On that note, if the leading numbers on the filename aren't helpful, feel free to get rid of them.
  5. That left nav has gotten really long. I don't think this is a problem for you to solve but it might be time to bring in some JS and get some collapsing going.

I'm going to do a pass through and tweak some grammar (curse my newspaper editor past) and send a PR over (hopefully tomorrow), but I think we're almost ready to merge this in and get it out into the world!

@mcsheffrey
Copy link
Contributor Author

Thanks for the great feedback Paul.

  1. Yeah, Tips aligns much better with the content of this section. Want me to go through and update those?
  2. Agreed.
  3. Yeah, we could change the spacing to better support multiple lines or since it's only two entry names that run over 1 line reword those. Thoughts? @chenglou
    screen shot 2013-11-04 at 6 02 46 pm
  4. I felt like the leading numbers in the filenames were helpful for keeping order. Easier to keep track of prev/next posts. Thoughts? @chenglou
  5. Agreed, left nav has gotten pretty long. I think the plan was to start grouping similiar entries under categories similiar to what Ember does. Or like you mentioned we could collapse each Docs category that isn't in use (see screenshot for example).

screen shot 2013-11-04 at 6 08 18 pm
screen shot 2013-11-04 at 6 10 43 pm

I'm excited to get this merged in! Again, appreciate of all your help and review.

@chenglou
Copy link
Contributor

chenglou commented Nov 5, 2013

  1. If we settled on calling it "tips" I'm fine with that.
  2. Fixed in a new PR!
  3. Ehh... ellipsis? Maybe not the greatest idea. Changing the spacing would be fine.
  4. The numbers do indeed help us for keeping tips in order, but I'm not sure how much they help beside that.
  5. Whatever we do now is a temp solution; collapsing still won't be enough in the future. I'll discuss more with @mcsheffrey.

mcsheffrey and others added 2 commits November 11, 2013 20:19
fix title case for tips everywhere
…cause it still referenced cookbooks. Updated some entry ID's (some side nav links didn't match entry permalinks)
@mcsheffrey
Copy link
Contributor Author

@zpao updated section Intro to remove Cookbook references. Anything else you think we ought to add to the Introduction?

@zpao
Copy link
Member

zpao commented Nov 12, 2013

I just opened mcsheffrey#20 for you with some edits. If you like them, then lets get this in :)

zpao added a commit that referenced this pull request Nov 14, 2013
@zpao zpao merged commit e6010bf into facebook:master Nov 14, 2013
@zpao
Copy link
Member

zpao commented Nov 14, 2013

Thanks for all of the hard work and patience!

zpao added a commit that referenced this pull request Nov 14, 2013
zpao added a commit that referenced this pull request Nov 14, 2013
bvaughn pushed a commit to bvaughn/react that referenced this pull request Aug 13, 2019
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.

5 participants