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

chore(Link): add react router example #605

Closed
wants to merge 1 commit into from

Conversation

havenchyk
Copy link
Contributor

FX-213

Description

Add an example of usage ReactRouter Link with picasso Link. Fixes #379

How to test

  • check out demo under Link -> Routing

Review

  • Annotate all props in component with documentation
  • Create examples for component
  • Ensure that deployed demo has expected results and good examples
  • Ensure that visuals specs are green See the documentation

@havenchyk havenchyk requested review from a team and ertrzyiks August 19, 2019 19:35
package.json Outdated Show resolved Hide resolved
@havenchyk havenchyk force-pushed the FX-213-add-typescript-in-stories branch 5 times, most recently from 7b17e5d to f449358 Compare August 20, 2019 18:32
@havenchyk havenchyk force-pushed the FX-213-add-link-router-example branch from 9283e79 to 8655b43 Compare August 20, 2019 20:50
@havenchyk havenchyk changed the base branch from FX-213-add-typescript-in-stories to fx-move-to-v4 August 20, 2019 20:51
@toptal-devbot
Copy link
Collaborator

🎉 Last commit is successfully deployed 🎉

Demo is available on:

Your davinci-bot 🚀


type PicassoRouterLink = RouterLinkProps & ComponentProps<typeof PicassoLink>

const Link = (props: PicassoRouterLink) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we didn't have this Link wrapper - would TS complain about the incorrect interface being used? I mean if we just used it inline:

<PicassoLink underline='always' as={RouterLink} to='/'>
  Home
</PicassoLink >

Copy link
Contributor Author

@havenchyk havenchyk Aug 21, 2019

Choose a reason for hiding this comment

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

PicassoLink doesn't have to property, that's why we need a wrapper

Copy link
Collaborator

Choose a reason for hiding this comment

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

it looks really weird as a simple example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how do you imagine this example to be simplified? @denieler

@@ -1,31 +0,0 @@
"use strict";
Copy link
Collaborator

Choose a reason for hiding this comment

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

not needed probably in this PR :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, it's done by mistake, sorry

@MilosMosovsky MilosMosovsky force-pushed the fx-move-to-v4 branch 8 times, most recently from e5aa2ed to bebea62 Compare August 23, 2019 13:12
@havenchyk havenchyk changed the base branch from fx-move-to-v4 to master August 26, 2019 13:27
@havenchyk havenchyk force-pushed the FX-213-add-link-router-example branch from 8655b43 to 8b39bd8 Compare August 26, 2019 13:40
@havenchyk
Copy link
Contributor Author

closing for now, will be added later

@havenchyk havenchyk closed this Aug 27, 2019
@havenchyk havenchyk deleted the FX-213-add-link-router-example branch October 11, 2019 15:10
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.

Enhance integration between Link and react-router
4 participants