Skip to content

Conversation

@SaraVieira
Copy link
Contributor

Closes CSB-79

@SaraVieira SaraVieira requested a review from siddharthkp April 20, 2020 22:50
@linear
Copy link

linear bot commented Apr 20, 2020

@lbogdan
Copy link
Contributor

lbogdan commented Apr 20, 2020

Build for latest commit cfef2cb is at https://pr3952.build.csb.dev/s/new.

@SaraVieira
Copy link
Contributor Author

Visual regressions fail because of limit

@SaraVieira SaraVieira changed the title add skeleton add skeleton to templates and start Apr 20, 2020
ITextProps;
ITextProps & {
as?: any;
to?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if this is the best way to do it, but we'll find out 🤷

@siddharthkp
Copy link
Contributor

Visual regressions fail because of limit

sad. let's find a way to run them only when needed for next month cc @lbogdan

@siddharthkp siddharthkp merged commit f27c926 into master Apr 21, 2020
@siddharthkp siddharthkp deleted the sara/csb-78-make-skeleton-for-start-page branch April 21, 2020 08:42
);

// export default withRouter(Content);
export const Content = withRouter(ContentComponent);
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for withRouter here, since we're not using any of the passed props.
https://reacttraining.com/react-router/core/api/withRouter

Suggested change
export const Content = withRouter(ContentComponent);

// <Redirect to="/dashboard/recent" />
// </Switch>
// );
const ContentComponent = () => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const ContentComponent = () => (
export const Content: FunctionComponent = () => (

@@ -1,5 +1,5 @@
// import React from 'react';
// import { Route, Switch, Redirect, withRouter } from 'react-router-dom';
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import React from 'react';
import React, { FunctionComponent } from 'react';

export const SandboxCard = ({ sandbox }) => (
<Element>
<Link href={sandboxUrl({ id: sandbox.id, alias: sandbox.alias })}>
{sandbox.title || sandbox.alias || sandbox.id}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use getSandboxName from common/utils instead

Suggested change
{sandbox.title || sandbox.alias || sandbox.id}
{getSandboxName(sandbox)}

@@ -0,0 +1,11 @@
import React from 'react';
import { sandboxUrl } from '@codesandbox/common/lib/utils/url-generator';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { sandboxUrl } from '@codesandbox/common/lib/utils/url-generator';
import { getSandboxName } from '@codesandbox/common/lib/utils/get-sandbox-name';
import { sandboxUrl } from '@codesandbox/common/lib/utils/url-generator';

return <Text>loading</Text>;
}

const sandboxes = data && data.me && data.me.sandboxes;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we know for sure there's no error or loading state, we know data.me.sandboxes is present

Suggested change
const sandboxes = data && data.me && data.me.sandboxes;
const sandboxes = data.me.sandboxes;

Comment on lines +53 to +56
const templates =
templatesData &&
templatesData.me &&
templatesData.me.recentlyUsedTemplates.slice(0, 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we know for sure there's no error or loading state, we know templatesData.me.recentlyUsedTemplates is present

Suggested change
const templates =
templatesData &&
templatesData.me &&
templatesData.me.recentlyUsedTemplates.slice(0, 4);
const templates = templatesData.me.recentlyUsedTemplates.slice(0, 4);

return <Text>loading</Text>;
}

const templates = data && data.me && data.me.templates;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we know for sure there's no error or loading state, we know data.me.templates is present

Suggested change
const templates = data && data.me && data.me.templates;
const templates = data.me.templates;

// import React from 'react';
// import { Route, Switch, Redirect, withRouter } from 'react-router-dom';
import React from 'react';
import { Route, Switch, Redirect, withRouter } from 'react-router-dom';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { Route, Switch, Redirect, withRouter } from 'react-router-dom';
import { Redirect, Route, Switch } from 'react-router-dom';

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