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.

Sign up for GitHub

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

Jump to bottom

Updated ESLint version & rules, then fix code across project, and fixed #212 #213

Merged
merged 8 commits into from
Mar 13, 2017
Merged

Updated ESLint version & rules, then fix code across project, and fixed #212 #213

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7b56930
Set .eslintrc to show lint warning/error
haxpor Mar 11, 2017
e9be6e4
Merge branch 'fix_eslint'
haxpor Mar 11, 2017
08e50f4
Update ESLint v. and its rules and fix warnings/errors across project
haxpor Mar 11, 2017
96c831b
Merge branch 'update_eslint'
haxpor Mar 11, 2017
d38636c
Fix issue 212
haxpor Mar 11, 2017
a5757f2
Merge branch 'fix_#212'
haxpor Mar 12, 2017
b0ba6d1
Review and fix #212 with better approach
haxpor Mar 12, 2017
ec45e68
Merge branch 'fix_#212'
haxpor Mar 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/components/grid/grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import classNames from 'classnames';
import GridIcon from './grid_icon';
import GridLabel from './grid_label';
import { Link } from 'react-router';
Copy link
Collaborator

Choose a reason for hiding this comment

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

@haxpor Thanks for the PR and work to the lint!

I believe to use Link in this case will require dependency from react-router and not everyone use react-router as router even though most of the single page application do.

Can you modify it to something like Cell that people will be able to pass into their component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree! That's much more robust.

I've fixed it to provide component property to be set in Grid as similar to Cell class.


/**
* WeUI Grid wrapper, contain elements of `GridIcon` and `GridLabel`
Expand Down Expand Up @@ -33,11 +34,11 @@ export default class Grid extends React.Component {
}, className);

return (
<a className={cls} {...others}>
<Link className={cls} {...others}>
{icon ? <GridIcon>{icon}</GridIcon> : false}
{children}
{label ? <GridLabel>{label}</GridLabel> : false}
</a>
</Link>
);
}
};