react-cosmos / rfcs

Change requests for React Cosmos
MIT License
1 stars 2 forks source link

Group components in a single view #2

Open ovidiubute opened 7 years ago

ovidiubute commented 7 years ago

What's up?

Non-technical people have trouble navigating CP if your project has a lot of components and fixtures. Ideally they'd like to be able to see similar components grouped together on one page, or have all of the fixtures in a single page for one component.

Mkay, tell me more...

I'm thinking we have a tree structure in the left nav menu, where if you click on the directory name you'll get a page with all of the components within it rendered under a specific fixture (this could be configurable, let's say that all of your components have a base.js, this would be configured in cosmos.config.js).

In addition to that, if you click on the name of a component, you should see a view of your component rendered in all states (or a subset of them). This would also be configurable, maybe we say render 3 fixtures per component, or maybe, wait for it: specify which fixtures are the most interesting to render (effectively showcasing your component in the most interesting states).

Obviously the pain points here are going to be:

There are more questions here but I'd love to hear your opinion on this, @skidding. I think it's a worthwhile investment to have this feature and it's something I've seen in other playgrounds as well, the difference being that they all require the users to write JSX, which is very difficult to maintain in the long term.

ovidiuch commented 7 years ago

Love it.

Extracting the directory from the name of the components

This can already be done. Most of the time automatically, but we also have getComponentName when componentPaths contains file paths (which probably should be made to work for all cases, as @valer-cara suggested in a private thread)

Rendering more than one fixture into CP at a time (what happens to modals, for example, which render on the body and take over your screen?)

Very good question. I'm sure there's a simple answer somewhere that we didn't find yet.

Here's an idea for the mix:

When you click on a component you see all the fixtures one after another, sorted alphabetically. But you could have a magic __fixtures__/COMPONENT_NAME/_list.js fixture that exports an array of fixture names, which allows you to only include the fixtures you want in the desired order. Later, this _list.js fixture could be generated from the UI by toggling/drag-n-droping (2020 🚀 ).

maciej-ka commented 7 years ago

Here is some progress to display all fixtures of a component in a single view: https://github.com/react-cosmos/react-cosmos/pull/371. Components can be clicked in the menu. The route used as a component page has a component part and no fixture part "?component=Counter".

I'm making a plan for rendering several instances on one page.

Currently component-playground.jsx calls this.sendFixtureToLoader() which transports one fixture to Loader.jsx. If I understand correctly, because Loader lives in an iframe, it has separate DOM and this makes it harder for him to access fixtures, so he receives them by listening to fixture-loaded event.

I think we could transport more than one fixture and introduce a new component that translates them into a list of loaders with single fixtures.

img_0013

On component click, we send all fixtures to Layouter and he will render them one by one. In a next step, we could have all components view, which would require introducing a default fixture.

My biggest worry is that we may need more complex routing system at some point.

ovidiuch commented 7 years ago

@maciej-ka so cool you started working on this!

I wish I were more transparent on this, but I'm actually working hard on react-cosmos/react-cosmos#364 and ended up refactoring both Playground and Loader. Component Playground has very old code so it was always something to avoid, especially since the tests are convoluted and use the Mocha/Karma setup. Besides what I already pushed on 360-separate-playground, I completely rewrote the Loader to be more robust and have atomic tests and am now in the progress of bringing the Component Playground to 2017 (Jest tests and split in smaller, better designed components). It's WIP so I didn't push code that isn't proper, but I'm hoping to do so this week. To have an idea, this is what the Playground-Loader communication looks like on my local branch:


Playground ⇆ Loader communication

The Cosmos UI is built out of two frames (both conceptually but also literally–components are loaded inside an iframe for full encapsulation). Because the Playground and the Loader aren't part of the same frame, we use postMessage to communicate back and forth.

From Playground to Loader:

From Loader to Playground:

Order of events at init:

  1. Playground renders in loading state and Loader <iframe> is added to DOM
  2. Loader renders inside iframe and sends loaderReady event to window.parent, along with user fixture list
  3. Playground receives loaderReady event, puts fixture list in state and exists the loading state

Order of events on selecting fixture:

  1. Playground sends fixtureSelect event to Loader with the selected component + fixture pair
  2. Loader receives fixtureSelect and renders corresponding component fixture (wrapped in user configured Proxy chain)
  3. User component renders, callback ref is bubbled up to Loader and fixtureLoad event is sent to Playground together with the serializable body of the selected fixture
  4. Playground receives serializable fixture body, puts it in state and uses it as the JSON contents of the fixture editor

A coupe of thoughts for this issue:

I hate to have to say this, but you should probably pause your work on this until we finish react-cosmos/react-cosmos#364. Would you be interested in helping with reviewing it when it's ready?

maciej-ka commented 7 years ago

Sure, I will code review, probably just to dive into the related code. I will remove WIP PR for now. Thanks for the extended info about Playground - Loader protocol!

maciej-ka commented 7 years ago

Hi @skidding, I've seen activity on a branch that you linked. Is it possible to pass many fixtures now and work on this (group components) task?

ovidiuch commented 7 years ago

Hi @maciej-ka,

Unfortunately (actually fortunately since it's going to be great :D), I went down the rabbit whole and ended up rewriting a lot of the old code to make it more robust, easier to understand and extend by contributors. See the highlights in this comment.

While the branch is not ready to be merged, you should be able to start the groundwork for this as it requires some amount of planning and high level design. Just keep in mind that the CP is 100% rewritten on my branch, so best to take a look over it before coming up with any code.

To answer your question, when I finish work on my branch we'll still not have a way to load more fixtures at once, so we'll need to come up with a solution for that here. It's relevant to note that the communication has changed and the Loader has the fixture contents now, while the Playground is merely a remote control that has acess to component and fixture names. The Loader actually wires the serializable fixture contents to the Playground in order for it to be displayed in the fixture editor.

This said, here are the rough steps I see included in this feature:

This should be enough to start the conversation while I'm still working on finishing the other branch.

@maciej-ka what do you think? See any flaws in the plan above? Any additional ideas or mentions?

PS. I wouldn't worry about the router because the current router simply maps the querystring params to props, which is ugly but straightforward. Or did you have a specific concern related to routing?

ovidiuch commented 7 years ago

@maciej-ka react-cosmos/react-cosmos#364 is merged. You're no longer blocked!

DJTB commented 7 years ago

Oh I'd love this...

I currently have an Icon component that renders an svg based on name prop (alongside size, color etc) pulling the relevant data from a constants file.

  ADD: {
    viewBox: '0 0 24 24',
    path: 'M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm5 11h-4v4h-2v-4H7v-2h4V7h2v4h4v2z',
  },

Having all the possible icons on one screen would be fantastic.

However, could this proposal include the extra ability to define multiple fixtures in a single file? I'm currently having to generate a whole heap of single fixtures instead:

import { ICONS } from './constants';

Object.keys(ICONS).forEach((name) => {
  const fileName = `__fixtures__/${name.toLowerCase()}.js`;
  const content = `
    export default {
      props: {
        name: '${name}',
        size: '5rem',
      },
    };`;
  fs.writeFile(fileName, content, (err) => err ? console.error(err) : console.log(`${fileName} generated`));
});
ovidiuch commented 7 years ago

However, could this proposal include the extra ability to define multiple fixtures in a single file? I'm currently having to generate a whole heap of single fixtures instead:

This would be a sweet! It's related to this, but can be implemented separately, after we have the functionality to render all fixtures of a component in a single page.

An elegant way would be to allow fixture files to export a list of objects, treating each of those objects as a separate fixture and rendering them in the new page being developed in this PR.

ovidiubute commented 7 years ago

My 2 cents: you could have a convention to name a fixture the same thing as the file that contains your React Component. That would mean it exports multiple fixtures. In order to still support multiple fixture names I would export an ES6 Map out of the fixture to easily determine at runtime if the fixture is a "new" fixture or not in order to keep backwards compatibility and not have to export weird domain specific objects or Arrays of fixtures.

ovidiuch commented 7 years ago

My 2 cents: you could have a convention to name a fixture the same thing as the file that contains your React Component. That would mean it exports multiple fixtures. In order to still support multiple fixture names I would export an ES6 Map out of the fixture to easily determine at runtime if the fixture is a "new" fixture or not in order to keep backwards compatibility and not have to export weird domain specific objects or Arrays of fixtures.

Hmm, why not just export a bare [] (prettier to the eye) and internally distinguish single vs multiple fixtures file by Array.isArray()? A single fixture always returns an object, never an Array.

Eg.

// Single fixture
export default {
  props: {
    foo: 'bar'
  }
}

// Multi fixture
export default [
  {
    props: {
      foo: 'bar'
    }
  },
  {
    props: {
      baz: 'qux'
    }
  }
]
ovidiubute commented 7 years ago

Because you've just lost the ability to give your fixtures friendly names. With a Map the key used could represent the name.

// Single fixture
const fixtures = new Map();

fixtures.set('with-data-type-null', {
  'data-type': null
});
fixtures.set('with-data-type-3', {
  'data-type': 3
});

export default fixtures;

Later on...

fixtures instanceof Map

true

NiGhTTraX commented 7 years ago

Because you've just lost the ability to give your fixtures friendly names

Why not add a title attribute to the fixture object? This would work for single fixtures as well.

ovidiuch commented 7 years ago

Interesting. I didn't imagine each fixture from a multi fixture file to have a name. 🤔

It sounds cool, but we need to see it as a whole. What @maciej-ka is working now is to put all existing single fixtures in a page.

So if you had

fixture1.js
fixture2.js

You'll have a page like this

Component Foo

fixture1
[ iframe ]

fixture2
[ iframe ]

This means we render fixture names from file names. @ovidiubute you're proposing a way to name more fixtures in the same file. My only worry is how these two types of naming fit together.

ovidiuch commented 7 years ago

Why not add a title attribute to the fixture object? This would work for single fixtures as well.

This is also interesting. It means you could have friendlier names for single fixture files that show up in the component page (where all fixtures are listed).

And you have a unified way of naming fixtures...

ovidiuch commented 7 years ago

Big fan of @NiGhTTraX's idea of fixture.title. Morover, I'd also like to see fixture.description, an optional paragraph describing the use case that serves as A. code documentation and B. UI documentation.

ovidiubute commented 7 years ago

I think at this point you've gone so far off the initial idea of a fixture that this proposed format is basically a Cosmos specific DSL. Props are already split into their own property name, which may or may not be popular with users, but metadata like title/description is taking it a bit too far, in my opinion. Once you've added sufficient Cosmos data into a fixture what is the point at which you need to maintain the fixture itself? Can you still reliably use it in a unit-test?

ovidiuch commented 7 years ago

I think at this point you've gone so far off the initial idea of a fixture that this proposed format is basically a Cosmos specific DSL.

You could say so. You have fixture.url with the Router proxy, and fixture.fetch with Fetch proxy, so you could say the Cosmos fixture is a (pluggable) DSL. I think the potential for a general purpose UI dev tool is very promising. I agree intricate fixtures can turn into hell, but I'd argue than if your fixture gets out of hand the component it mocks might also have too much responsibility.

Once you've added sufficient Cosmos data into a fixture what is the point at which you need to maintain the fixture itself?

Hard to judge but I see your point. The question I think is if the benefits outweigh the cost.

Can you still reliably use it in a unit-test?

As long as it's deterministic, I say yes. But truth be told Cosmos evolved into more of an integration tool. Depends on what you see as a unit. Is withRouter(connect(Component)) a unit?

But we've strayed from the initial topic and we should open different threads to explore these avenues.

alp82 commented 6 years ago

Not sure if i followed everything in here correctly, but i am a huge fan of grouping fixtures on one page. This would be useful for:

By the way, with version 3 this should be already possible with an ugly workaround. What you need is a component that is basically a layout wrapper (css flex or grid) which gets loaded in the fixture. Then you define an arbitrary complex component structure as children in the fixture (be sure to import React from 'react').

Example:

import React from 'react';
import Flex from 'my/grid/Flex';
import Icon from 'my/icon/Icon';

export default {
  component: Flex,
  children: [
    <Icon key="1" name="search" />,
    <Icon key="2" name="comment" />,
  ],
};

Since you write your own markup, you can layout everything as you want. But again, this is an ugly hack.