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: added properties from react-router link component #74

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mimitorres
Copy link
Contributor

Ticket/Card

Description of the change

  • Added props from react-router Link to our wrapper.

Screenshot/Execution

N/A

Related PRs

N/A

@@ -13,41 +13,55 @@ import type { Params, RouteName } from "../../routes/routes";
defined in this project. To link outside, use <a></a> tags as usual.
*/

type Route = {
pathParams?: Params;
Copy link
Contributor

Choose a reason for hiding this comment

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

are we intentionally changing spaces to tabs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, it was a misconfiguration on my code editor 😭

@mimitorres mimitorres force-pushed the chore-redesign-props-link branch from 9005284 to 088be2c Compare July 22, 2024 13:58
@mimitorres mimitorres force-pushed the chore-redesign-props-link branch from 9c5c44f to 9e81d6b Compare July 22, 2024 15:24
@mimitorres mimitorres requested a review from mlvieras August 12, 2024 18:50
// Extract pathParams from the routeName
type AppLinkProps<R extends RouteName> = {
type AppLinkProps = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if it makes sense to do something like interface AppLinkProps extends <whatever the Link props type is>

@@ -13,26 +13,24 @@ import type { Params, RouteName } from "../../routes/routes";
defined in this project. To link outside, use <a></a> tags as usual.
*/

type AppRedirectProps<R extends RouteName> = {
type Route = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This type is duplicated, I'd prefer to have it centralized if possible

@chaba11 chaba11 requested a review from Copilot November 18, 2024 18:58

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no suggestions.

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.

3 participants