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.

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

Fix #212 #214

Closed
wants to merge 5 commits into from
Closed

Fix #212 #214

wants to merge 5 commits into from

Conversation

haxpor
Copy link
Contributor

@haxpor haxpor commented Mar 11, 2017

I branched out from my forked repository, and manually tested it. It works fine and can provide normal <a> functionality to link to normal URL, or react-router's URL.

We can use the following code in Grid's Data.

const data = [
  {
    icon: <img src={issue_icon} alt="issue icon"/>,
    label: 'Issues ...',
    to: '/issues'
  },
  {
    icon: <img src={project_icon} alt="project icon"/>,
    label: 'Projects',
    to: '/projects'
  },
  {
    icon: <img src={contribute_icon} alt="contribute icon"/>,
    label: 'Contributes',
    href: 'https://google.com'
  }
];

The first two items are for react-router's URL, and the third one is for normal URL. When click, it takes to destination properly.

Anyway, the code seems easy and straightforward but not sure it should be the approach use or not as it directly use react-router's <Link> and I checked react-weui already depends on it although older version.

The reason is that starting from version 1.0 of ESLint, all linting
rules are turned off by default.
- already tested via `npm run test` and all tests passed.
- most of code fixed across the project are about semi colon, space
  after if/else if/else/for statement, removed newline at the end of file, a few dangerous code statement fixed, and a few that is not needed.
- ESlint is updated to version 3.17.1. The project has used version
  1.x.x. This should be better.
- ESLint's rules are updated to be more stricted but not too strict. See
  it in .eslintrc.
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 82.097% when pulling d38636c on haxpor:fix_#212 into ace7de6 on weui:master.

@haxpor
Copy link
Contributor Author

haxpor commented Mar 12, 2017

closed due to branch management, and duplicated in #213

@haxpor haxpor closed this Mar 12, 2017
@haxpor haxpor deleted the fix_#212 branch March 12, 2017 05:33
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.

None yet

2 participants