This is a small thing, but JSX allows you to use literal strings as attribute values without the expression braces. You have to use double quotes, so this could be:

<CategoryList title="Shop by category" id={2} />

Soon we will create an internationalization system that finds and replaces these strings, so they should use a common syntax.

@jimbo would give better advice here, but I think you might want to use a fr unit instead of 6rem, if you want a predictable number of items in each row.

We have mobile, tablet, desktop view.
My idea was to have fixed width of items + margin between them.
Center aligned, no matter which device we are talking about.

If you have some other solution please let me know.

The approach used here achieves the design they're aiming for, so it looks good to me. Thanks for embracing grid. 👍

Thank you for adding this TODO comment. This really helps everyone. 👍

You should not need to hardcode the full domain! Relative paths from root should be fine. Are you working with Luma sample data?

Yep, I use magento 2.3-develop + sampledata

Same here; this variable should probably be pub/media.

Excellent work implementing a public CSS classes API.

This component probably shouldn't have a default ID. I'm guessing you made this for easy testing, but it should be removed.

Idea was to have preset of the default magento category.
"Root Catalog" - id="1"
"Default Category" - id="2"

We have developed a pattern for nullable props that should conditionally render a portion of JSX. (@jimbo and I should probably document this pattern). This could be another component, but it seems too single-purpose; our pattern is to create a class getter that encapsulates this logic. An example here shows a property {this.options} that you can simply embed in your JSX with no conditional, because the conditional is implemented in the getter.

Here, you should add a class method:

get header() {
  const { title, classes} = this.props;
  return title ? (
    <div className={classes.header}>
        <h2 className={classes.title}>
            <span>{title}</span>
        </h2>
    </div>
  ) : null;
}

And then in your render method right here, you replace this expression with

<div className={classes.root>
  {this.header}
  <Query query={categoryListQuery} variables= {{ id }}>

(We recommend a getter instead of a plain accessor function, because a getter prevents you from passing arguments internally, which could put logic where it doesn't belong. A getter ensures that the method must have the same signature as render(), so that it could easily be moved into another component if necessary.)

How would you feel about implementing the inner function of .map here as another Component instead of inline JSX? It's not mandatory, but it's something to think about.

Yep, awesome idea.

I know we've merged this already, but yes—in general we try to avoid performing work inside JSX expressions. Eventually, I'd like to cover that with a lint rule. Certainly not a dealbreaker, though, so thanks for your understanding. 😄