Skip to content

Fix tutorial journey and typos #71

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

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

Fix tutorial journey and typos #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f03254d
fix: small wording update for README
HeVictor Dec 27, 2022
8e7ccad
fix: fix broken tutorial journey using updated react-router-dom impl …
HeVictor Jan 2, 2023
9fdc421
fix: removes unused setSearchParams param
HeVictor Jan 3, 2023
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ You can test the website locally using `yarn start`.

<h2 id="deployment">Deployment</h2>

You can the website to GitHub pages using `npm run deploy`.
You can deploy the website to GitHub pages using `npm run deploy`.

<h2 id="contributing">Contributing</h2>

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"proptypes": "^1.1.0",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react-router-dom": "^6.2.1",
"react-router-dom": "^6.4.5",
"react-scripts": "5.0.0",
"react-transition-group": "^4.4.2"
},
Expand Down
18 changes: 12 additions & 6 deletions src/components/UserMessage.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,31 @@
import React, { useState, useEffect } from 'react';
import { Link, useSearchParams } from 'react-router-dom';
import user from '../img/user-img.png';
import { UserResponseList } from './UserResponseList';
import urlToJSON from './utils/urlToJSON';
import { Link } from 'react-router-dom';
import BackIcon from '../img/back.png';
import createPathIdQuery from './utils/createPathIdQuery';

export default function UserMessage({ setjsonPathMap, jsonPathMap, pathId }) {
export default function UserMessage({ setjsonPathMap, jsonPathMap }) {
const [userName, setuserName] = useState('Test-User >');

const [searchParams] = useSearchParams();
const pathId = searchParams.get('pathId');
Copy link
Contributor Author

@HeVictor HeVictor Jan 3, 2023

Choose a reason for hiding this comment

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

From what I can tell in the previous implementation I'm guessing pathId is used as a search parameter. In react-router-dom v6 the new way to get search parameters is useSearchParams so I updated the code to use this to fetch the actual 'pathId' value


useEffect(() => {
setjsonPathMap(urlToJSON(pathId));
urlToJSON(pathId);
setuserName('You');
}, [pathId, setjsonPathMap]);

return (
<div className='user-message-section scroll'>
{/* Back button link */}
{/* when pathId===undefined (i.e. First Message) --> Don't render back buttom. */}
{pathId !== undefined ? (
{/* when pathId is falsy (i.e. First Message) --> Don't render back buttom. */}
{pathId ? (
<Link
to={String(pathId).substring(0, String(pathId).length - 2)}
to={
createPathIdQuery(String(pathId).substring(0, String(pathId).length - 2))
}
className='back-button'
>
<img src={BackIcon} alt='back' className={'back-img'}></img>
Expand Down
52 changes: 27 additions & 25 deletions src/components/UserMessageSection.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,34 @@
import React from 'react';
import PathMap from './PathMap';
import { Routes, Route, HashRouter as Router } from 'react-router-dom';
import {
Routes,
Route,
BrowserRouter as Router,
createBrowserRouter,
RouterProvider,
createRoutesFromElements,
} from 'react-router-dom';
import UserMessage from './UserMessage';

export function UserMessageSection({ setjsonPathMap, jsonPathMap }) {
return (
<Router>
<Route
render={({ location }) => (
<Routes location={location}>
<Route
// pathId - URL param for directly accessing particular path.
path='/:pathId?'
component={({ match }) => (
<div className={'fade-in-to-right'}>
<UserMessage
pathId={match.params.pathId}
setjsonPathMap={(json) => setjsonPathMap(json)}
jsonPathMap={jsonPathMap}
/>
</div>
)}
/>

<Route path='/map' component={PathMap} />
</Routes>
)}
/>
</Router>
const router = createBrowserRouter(
createRoutesFromElements(
<>
<Route path="/map" element={PathMap} />
<Route
path="/AdventureSite"
Copy link
Contributor Author

@HeVictor HeVictor Jan 3, 2023

Choose a reason for hiding this comment

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

I might need some clarification here, from yarn start I see that the URL that the app launches on is always http://localhost:3000/AdventureSite which is why I needed to specify the path to be /AdventureSite here so that the app renders the tutorial journey properly on startup. I'm unsure if this will work as-is with how the site is rendered on the Terasology website itself, especially as the link there uses a hash in the URL but the implementation here has been updated to use a BrowserRouter

element={
<div className={'fade-in-to-right'}>
<UserMessage
setjsonPathMap={(json) => setjsonPathMap(json)}
jsonPathMap={jsonPathMap}
/>
</div>
}
/>
</>
)
Copy link
Contributor Author

@HeVictor HeVictor Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The original implementation was giving an error that complained <Route> component was not being wrapped by a <Routes> component. While adding another <Routes> component on top does solve the issue like the error message said, I've decided to change the implementation to what's recommended by the react-router-dom v6 docs that utilises the createRoutesFromElements function which also converts this app to use a BrowserRouter instead of a HashRouter

);

return <RouterProvider router={router} />;
}
12 changes: 8 additions & 4 deletions src/components/UserResponse.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React, { useState, useEffect } from 'react';
import { Link } from 'react-router-dom';
import createPathIdQuery from './utils/createPathIdQuery';

//Each UserResponse
export function UserResponse({ object, index, pathId }) {
//url to visit on user selection
const [url, setURL] = useState('');

useEffect(() => {
if (pathId === undefined) {
if (!pathId) {
setURL('');
} else {
setURL(pathId);
Expand All @@ -24,12 +25,15 @@ export function UserResponse({ object, index, pathId }) {
}
to={
object.child !== undefined
? url.toString() + 'u' + (Number(index) - 1).toString()
? createPathIdQuery(
url.toString() +
'u' +
(Number(index) - 1).toString())
: object.jump !== undefined
? object.jump.toString()
? createPathIdQuery(object.jump.toString())
: object.link === undefined
? '/'
: url.toString()
: createPathIdQuery(url.toString())
}
onClick={() => {
if (object.isGraph === true) {
Expand Down
6 changes: 5 additions & 1 deletion src/components/UserResponseList.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { UserResponse } from './UserResponse';
import { useState, useEffect } from 'react';

//List of the User Responses in User Section
export function UserResponseList({ setjsonPathMap, jsonPathMap, pathId }) {
export function UserResponseList({
setjsonPathMap,
jsonPathMap,
pathId,
}) {
const [data, setData] = useState(jsonPathMap);

useEffect(() => {
Expand Down
5 changes: 5 additions & 0 deletions src/components/utils/createPathIdQuery.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { PATH_ID_QUERY_PARAM } from '../../consts/URLConsts';

export default function createPathIdQuery(paramValue) {
return PATH_ID_QUERY_PARAM + paramValue;
}
Copy link
Contributor Author

@HeVictor HeVictor Jan 3, 2023

Choose a reason for hiding this comment

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

As I wanted to avoid making huge underlying changes I've made the existing Link components from the different tutorial journey options update the pathId query parameters by simply appending the end of the URL with ?pathId={paramValue} as shown by the util function here. There is a way to use the setSearchParams function from the v6 'react-router-dom' but that might require more changes to use something other than Link components so I've done it this way for now

2 changes: 1 addition & 1 deletion src/components/utils/urlToJSON.jsx → src/components/utils/urlToJSON.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import data from '../../data/pathMap';
//Take pathId as a parameter and returns JSON data from pathMap.
export default function urlToJSON(u) {
//url format - u[position]-u[position]
var url = String(u).split('u');
var d = data;

if (u === null || u === undefined) {
return d;
}

var url = String(u).split('u');
url.forEach((i, index) => {
if (d === undefined || index > 100) {
d = data;
Expand Down
1 change: 1 addition & 0 deletions src/consts/URLConsts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const PATH_ID_QUERY_PARAM = '?pathId=';
37 changes: 17 additions & 20 deletions src/data/ContributionResources/Git.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
import ContributionResources from ".";

export default {
'user-responses': [
{
name: 'How to work with Git?',
link: 'http://learngitbranching.js.org/',
},
{
name: 'How to work with forks?',
link: 'https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/working-with-forks'
},
{
name: 'Done. What else should I know about?',
child: ContributionResources,
},
],
'gooey-response': {
gooey: 'New to Git / GitHub? Check this out.',
'user-responses': [
{
name: 'How to work with Git?',
link: 'http://learngitbranching.js.org/',
},
};

{
name: 'How to work with forks?',
link: 'https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/working-with-forks',
},
{
name: 'Done. What else should I know about?',
jump: 'u0u2u0u0',
Copy link
Contributor Author

@HeVictor HeVictor Jan 3, 2023

Choose a reason for hiding this comment

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

This particular component was causing a circular dependency problem that crashed the app on startup as it referred to the ContributionResources which in turn referred back to this Git component. I've fixed this by replacing the 'child' param with the equivalent 'jump' param 'pathId' value that points to the same expected page

},
],
'gooey-response': {
gooey: 'New to Git / GitHub? Check this out.',
},
};
9 changes: 4 additions & 5 deletions src/data/ContributionResources/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Git from "./Git";
import Conventions from "./Conventions";
import Workspace from "./Workspace";
import Contributor from "../Terasology/Contributor";
import Git from './Git';
import Conventions from './Conventions';
import Workspace from './Workspace';

export default {
'user-responses': [
Expand All @@ -23,7 +22,7 @@ export default {
},
{
name: 'Which conventions should I follow?',
child: Conventions
child: Conventions,
},
{
name: 'Okay, got it. Now I feel ready to contribute!',
Expand Down
2 changes: 1 addition & 1 deletion src/data/pathMap.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default {
},
],
'gooey-response': {
gooey: 'Which project you wish to explore? 🧐 ',
gooey: 'Which project do you wish to explore? 🧐 ',
},
},
},
Expand Down
36 changes: 17 additions & 19 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,7 @@
core-js-pure "^3.20.2"
regenerator-runtime "^0.13.4"

"@babel/runtime@^7.10.2", "@babel/runtime@^7.11.2", "@babel/runtime@^7.12.5", "@babel/runtime@^7.16.3", "@babel/runtime@^7.7.6", "@babel/runtime@^7.9.2":
"@babel/runtime@^7.10.2", "@babel/runtime@^7.11.2", "@babel/runtime@^7.12.5", "@babel/runtime@^7.16.3", "@babel/runtime@^7.9.2":
version "7.17.2"
resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.17.2.tgz#66f68591605e59da47523c631416b18508779941"
integrity sha512-hzeyJyMA1YGdJTuWU0e/j4wKXrU4OMFvY2MSlaI9B7VQb0r5cxTE3EAIS2Q7Tn2RIcDkRvTA/v2JsAEhxe99uw==
Expand Down Expand Up @@ -1645,6 +1645,11 @@
schema-utils "^3.0.0"
source-map "^0.7.3"

"@remix-run/router@1.0.5":
version "1.0.5"
resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.0.5.tgz#d5c65626add4c3c185a89aa5bd38b1e42daec075"
integrity sha512-my0Mycd+jruq/1lQuO5LBB6WTlL/e8DTCYWp44DfMTDcXz8DcTlgF0ISaLsGewt+ctHN+yA8xMq3q/N7uWJPug==

"@rollup/plugin-babel@^5.2.0":
version "5.3.0"
resolved "https://registry.yarnpkg.com/@rollup/plugin-babel/-/plugin-babel-5.3.0.tgz#9cb1c5146ddd6a4968ad96f209c50c62f92f9879"
Expand Down Expand Up @@ -5243,13 +5248,6 @@ he@^1.2.0:
resolved "https://registry.yarnpkg.com/he/-/he-1.2.0.tgz#84ae65fa7eafb165fddb61566ae14baf05664f0f"
integrity sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==

history@^5.2.0:
version "5.2.0"
resolved "https://registry.yarnpkg.com/history/-/history-5.2.0.tgz#7cdd31cf9bac3c5d31f09c231c9928fad0007b7c"
integrity sha512-uPSF6lAJb3nSePJ43hN3eKj1dTWpN9gMod0ZssbFTIsen+WehTmEadgL+kg78xLJFdRfrrC//SavDzmRVdE+Ig==
dependencies:
"@babel/runtime" "^7.7.6"

hoopy@^0.1.4:
version "0.1.4"
resolved "https://registry.yarnpkg.com/hoopy/-/hoopy-0.1.4.tgz#609207d661100033a9a9402ad3dea677381c1b1d"
Expand Down Expand Up @@ -8333,20 +8331,20 @@ react-refresh@^0.11.0:
resolved "https://registry.yarnpkg.com/react-refresh/-/react-refresh-0.11.0.tgz#77198b944733f0f1f1a90e791de4541f9f074046"
integrity sha512-F27qZr8uUqwhWZboondsPx8tnC3Ct3SxZA3V5WyEvujRyyNv0VYPhoBg1gZ8/MV5tubQp76Trw8lTv9hzRBa+A==

react-router-dom@^6.2.1:
version "6.2.1"
resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-6.2.1.tgz#32ec81829152fbb8a7b045bf593a22eadf019bec"
integrity sha512-I6Zax+/TH/cZMDpj3/4Fl2eaNdcvoxxHoH1tYOREsQ22OKDYofGebrNm6CTPUcvLvZm63NL/vzCYdjf9CUhqmA==
react-router-dom@^6.4.5:
version "6.4.5"
resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-6.4.5.tgz#4fdb12efef4f3848c693a76afbeaed1f6ca02047"
integrity sha512-a7HsgikBR0wNfroBHcZUCd9+mLRqZS8R5U1Z1mzLWxFXEkUT3vR1XXmSIVoVpxVX8Bar0nQYYYc9Yipq8dWwAA==
dependencies:
history "^5.2.0"
react-router "6.2.1"
"@remix-run/router" "1.0.5"
react-router "6.4.5"

react-router@6.2.1:
version "6.2.1"
resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.2.1.tgz#be2a97a6006ce1d9123c28934e604faef51448a3"
integrity sha512-2fG0udBtxou9lXtK97eJeET2ki5//UWfQSl1rlJ7quwe6jrktK9FCCc8dQb5QY6jAv3jua8bBQRhhDOM/kVRsg==
react-router@6.4.5:
version "6.4.5"
resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.4.5.tgz#73f382af2c8b9a86d74e761a7c5fc3ce7cb0024d"
integrity sha512-1RQJ8bM70YEumHIlNUYc6mFfUDoWa5EgPDenK/fq0bxD8DYpQUi/S6Zoft+9DBrh2xmtg92N5HMAJgGWDhKJ5Q==
dependencies:
history "^5.2.0"
"@remix-run/router" "1.0.5"

react-scripts@5.0.0:
version "5.0.0"
Expand Down