qri-io / 2017-frontend

qri electron & web frontend
https://qri.io
MIT License
23 stars 7 forks source link

refactor(Dataset): change format of dataset component #370

Closed ramfox closed 6 years ago

ramfox commented 6 years ago

We are planning a refactor of the Dataset component, to orient around a top bar navigation.

components

Here are a list of effected components, and the sub components or main content for each:

DatasetHeader

DatasetSummary

Body

Meta

Structure

Viz

Transform

Commit

Render

Changes

navigation / paths

option One

The Dataset component will be refactored to contain a DatasetHeader, and a section that will load the correct component, depending on the path.

/handle/ds_name/at/network/hash/{section} /handle/ds_name/{section}

section can be: "meta" - Meta "body" - Body "viz" - Viz "Render" - Render "transform" - Transform "changes" - Changes "dataset" - DatasetSummary "transform" - Transform "structure" - Structure

Any other endings will display the DatasetSummary, including the /handle/ds_name/at/network/hash or /handle/ds_name paths

The corresponding name of each section will be bolded and underlined.

The decision about which sub-component to display will be handled by a switch function in the Dataset component. Each sub-component will have it's own render functions (eg, renderChanges()), one of will be returned by the switch function, based on the section passed from the router.

option two

The Dataset component contains a DatasetHeader (perhaps no longer necessary for it to be a separate component), and a space to render a child element that gets passed in by a wrapper component, which itself is wrapped in a container

Meta

All ill-formed paths 404 (no DatasetSummary catch all)

This is more rigid and might be safer, but it also feels more redundant with lots of repeat code. Also, feels like this may trigger way more re-renders.

EDIT:

This PR also:

ramfox commented 6 years ago

I'm proposing we distinguish between viz the rendered html output that comes from running a dataset through a template, and viz the template itself.

I've used Visualize and Render in different places to mean "the rendered html output", and Viz to mean "the template used when you render this dataset"

Render will make more sense to folks who use the command line qri render command and since it is literally the same output, this could be a nice parallel. Visualize might be more straight forward, though.

b5 commented 6 years ago

Big fan of distinguishing between viz and Render. Term wise, I do really like the notion of Render, which is a bias inherited from the film world, but it does feel right. I vote we go with Render until someone comes up with a better solution. I like the idea of render also coming from cooking (rendered fat), which gives the term a degree of permeability while maintaining accuracy.

b5 commented 6 years ago

In regard to routing, a few quick things.

Generally, option two seems more tenable long term to me. Some of these routes are going to get real complicated. (Body is going to have an entire spreadsheet component, /render is going to take up a user-arbitrary amount of resources, ps, I think we should rename the /visualize route to /render) . Cutting these up into separate components now sets us up for code splitting and other goodness in the future.

Secondly, I'd love these routes take advange of the nesting features offered by react router, where the DatasetContainer contains a DatasetHeader component (which should stay a separate component), but also has an area that renders this.props.children, which is dictated by adding a new layer of nesting to the router configuration.

ramfox commented 6 years ago

Made the initial refactor to Dataset to structure it as a DatasetHeader and a nested react router :)

I'm v excited about this, it's going to super un-bloat Dataset. I haven't yet removed any of the dead code, as I imagine I will need to reference some of it as I move forward in adding the routes.

Next step, I'm going to adjust the look of DatasetHeader to match our sketch file.

b5 commented 6 years ago

haven't had a chance to look, but let's keep deleting stuff from dataset as we go. I'd love to clean up the dataset component as part of this PR to the point that all code in that component is in fact in use.

ramfox commented 6 years ago

Next steps:

Dogfood peer's dataset page for weird behavior Dogfood search results dataset pages for weird behavior Add transfer spinner to dataset page and dataset item