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

How do we handle relative links in the Markdown descriptions? #4

Closed
cammoore opened this issue Nov 10, 2018 · 4 comments
Closed

How do we handle relative links in the Markdown descriptions? #4

cammoore opened this issue Nov 10, 2018 · 4 comments

Comments

@cammoore
Copy link
Contributor

Please do work for this task in a branch called issue-4.

The descriptions of some Career Goals have links to the related courses and other related Career Goals. These links should open an explorer showing the course or career goal.

@gcalica
Copy link
Contributor

gcalica commented Jun 5, 2019

I think the best way we can fix this problem is to not make them relative links. I think we should update and change the links of the markdown text in the database itself. We need to do this anyway because right now for the relative course links, they are [ICS 314](../courses/ics314). But even if we can make some react-router code to handle this, it needs to be [ICS 314](../courses/ics_314) because of the slug names anyway. So we need to change the markdown text from the database anyway.

So instead of [Game Developer](game-developer), it should be [Game Developer](/explorer/career-goals/game-developer).

And instead of [ICS 314](../courses/ics314, it should be [ICS 314](/explorer/courses/ics_314).

Another reason why we don't want them to be relative links is because the relativity would obviously only work for certain pages. For example in the case of Career Goals, besides the Explorer page, we also view the description of Career Goals in the Admin Data Model page. So even if we do end up making the relative links work FROM the Explorer Career Goals page, that doesn't necessarily mean the same code would then work FROM the Admin Data Model page. So at this point the relative links just makes things a hassle that we might have to make two possibly different code just to handle the relative links.

From this point, I already have a function that can deal with these types of links with react-router.
Taken from: remarkjs/react-markdown#29 (comment)

private routerLink = (props) => {
    const username = this.props.match.params.username;
    const baseUrl = this.props.match.url;
    const baseIndex = baseUrl.indexOf(username);
    const baseRoute = `${baseUrl.substring(0, baseIndex)}${username}`; 
    return (props.href.match(/^(https?:)?\/\//)
      ? <a href={props.href}>{props.children}</a>
      : <Link to={`${baseRoute}${props.href}`}>{props.children}</Link>);
}

...
render {
   ...
   <Markdown escapeHtml={true} source={description} renderers={{ link: this.routerLink }}/>
}

If we're viewing from some student account,

  • baseRoute returns /student/:username
  • If the markdown link is [Game Developer](/explorer/career-goals/game-developer), then ${baseRoute}${props.href} returns /student/:username/explorer/career-goals/game-developer which then gets routed properly.

Notes:
If we do end up going this method, right now there are two Collections that we need to change the markdown links for their description: CareerGoalCollection and FeedCollection. Obviously, the markdown links in the description field for CareerGoalCollection.

For FeedCollection, we need to change how the description is made for defineNewCourse(), defineNewOpportunity(), defineNewVerifiedOpportunity(), defineNewCourseReview(), defineNewOpportunityReview(), and updateVerifiedOpportunity() functions. Done in 2877f24

@gcalica
Copy link
Contributor

gcalica commented Jun 5, 2019

We should also probably make a separate file that exports the function that returns the baseRoute and the routerLink function from above. I see the code to get the baseRoute all over the source code of RadGrad2, so I think it is appropriate to put them into a utility/helper functions file that can be imported.

Done in #77

@gcalica
Copy link
Contributor

gcalica commented Jun 14, 2019

Also adding on top of this issue, clicking non-radgrad links should open that link into a new tab in your browser. This should be able to be easily be done by just modifying the <a> in the routerLink function

private routerLink = (props) => {
    const username = this.props.match.params.username;
    const baseUrl = this.props.match.url;
    const baseIndex = baseUrl.indexOf(username);
    const baseRoute = `${baseUrl.substring(0, baseIndex)}${username}`; 
    return (props.href.match(/^(https?:)?\/\//)
      ? <a href={props.href} target="_blank" rel="noopener noreferrer">{props.children}</a>
      : <Link to={`${baseRoute}${props.href}`}>{props.children}</Link>);
}

Done in #87

@gcalica
Copy link
Contributor

gcalica commented Jul 7, 2019

Done in #46 (5197afd)

@gcalica gcalica closed this as completed Jul 7, 2019
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

No branches or pull requests

2 participants