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.

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

Jump to bottom

convert Home page to use TypeScript #3124

Merged
merged 2 commits into from
Nov 20, 2023
Merged

convert Home page to use TypeScript #3124

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49b70ec
convert Home page to use TypeScript
mehrdadrafiee Nov 18, 2023
600424e
generic type for strings
mehrdadrafiee Nov 19, 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
12 changes: 4 additions & 8 deletions src/components/Home/Buttons.jsx → src/components/Home/Buttons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React from 'react';
import { Link } from 'react-router-dom';
import PropTypes from 'prop-types';
import FlatButton from 'material-ui/FlatButton';
import { connect } from 'react-redux';
import { IconSteam } from '../Icons';
import { ButtonsDiv } from './Styled';
import config from '../../config';

const Buttons = ({ user, strings }) => (
import { HomePageProps } from './Home';

const Buttons = ({ user, strings }: HomePageProps) => (
<ButtonsDiv>
{
!user &&
Expand All @@ -30,12 +31,7 @@ const Buttons = ({ user, strings }) => (
</ButtonsDiv>
);

Buttons.propTypes = {
user: PropTypes.shape({}),
strings: PropTypes.shape({}),
};

const mapStateToProps = (state) => {
const mapStateToProps = (state: any) => {
const { data } = state.app.metadata;
return {
user: data.user,
Expand Down
44 changes: 30 additions & 14 deletions src/components/Home/Home.jsx → src/components/Home/Home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,32 @@
import React from 'react';
import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import Buttons from './Buttons';
import Why from './Why';
import Sponsors from './Sponsors';
import { HeadContainerDiv, HeadlineDiv, DescriptionDiv, BottomTextDiv } from './Styled';

const Home = ({ strings }) => (
export interface HomePageProps {
user?: string
strings: {
Copy link
Member

Choose a reason for hiding this comment

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

I think it will be tedious to type all of the strings separately. . . we could just type it as the en-us JSON file type dict, or even just generic string dict

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like having a separate file only for types and importing what's needed? can you give an example please?

Copy link
Member

Choose a reason for hiding this comment

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

The easy solution is just to type strings as {[key: string]: string} which you can share across everything that's using the strings.

Alternatively, since the en-US.json file has all the keys in it, it should be possible to tell TS to construct a type out of that. That would be nice because you'll get autocomplete/checking if there's a typo, but I'm not 100% sure how to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so looks like there are 3 approaches to this:

  • just use a generic string dictionary (updated the PR to use this!)
  • use JSON file for type inference. this requires installing a JSON schema generator such as https://github.com/vega/ts-json-schema-generator. (I tried it but it didn't work because it only supports '.ts', '.tsx', '.d.ts', '.cts', '.d.cts', '.mts', '.d.mts' formats.)
  • create a separate interface for strings like so:
interface Strings {
  app_name: string;
  app_description: string;
  home_background_by: string;
  // ...
}

interface HomePageProps {
  user?: string;
  strings: Strings;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@howardchung let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe since the JSON files will be flat we can use keyof to generate a union of the possible key values instead of string?

https://www.typescriptlang.org/docs/handbook/2/keyof-types.html

But we can do that in a separate change

app_name: string
app_description: string
home_background_by: string
home_login: string
home_login_desc: string
home_parse: string
home_parse_desc: string
home_sponsored_by: string
home_become_sponsor: string
home_opensource_title: string
home_opensource_desc: string
home_indepth_title: string
home_indepth_desc: string
home_free_title: string
home_free_desc: string
}
}

const Home = ({ strings }: HomePageProps) => (
<div>
<HeadContainerDiv>
<HeadlineDiv>
Expand All @@ -15,29 +35,25 @@ const Home = ({ strings }) => (
<DescriptionDiv>
<h2>{strings.app_description}</h2>
</DescriptionDiv>
<Buttons />
<Buttons strings={strings} />
</HeadContainerDiv>
<Why />
<Sponsors />
<BottomTextDiv>
<span id="bg-image-description">{strings.home_background_by}</span>
<a
href="//www.artstation.com/artist/mikeazevedo"
target="_blank"
rel="noopener noreferrer"
aria-describedby="bg-image-description"
<a
href="//www.artstation.com/artist/mikeazevedo"
target="_blank"
rel="noopener noreferrer"
aria-describedby="bg-image-description"
aria-label="Mike Azevedo on artstation.com"
> Mike Azevedo
</a>
</a>
</BottomTextDiv>
</div>
);

Home.propTypes = {
strings: PropTypes.shape({}),
};

const mapStateToProps = state => ({
const mapStateToProps = (state: any) => ({
content: state.content,
strings: state.app.strings,
});
Expand Down
13 changes: 5 additions & 8 deletions src/components/Home/Sponsors.jsx → src/components/Home/Sponsors.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React from 'react';
import FlatButton from 'material-ui/FlatButton';
import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import styled from 'styled-components';
import { ButtonsDiv } from './Styled';
import config from '../../config';

import { HomePageProps } from './Home';

const StyledDiv = styled.div`
display: flex;
flex-direction: column;
Expand Down Expand Up @@ -34,11 +35,11 @@ const StyledDiv = styled.div`
}
`;

const Sponsors = ({ strings }) => (
const Sponsors = ({ strings }: HomePageProps) => (
<StyledDiv>
<div className="headline">{strings.home_sponsored_by}</div>
<div className="images">
{config.VITE_ENABLE_DOTA_COACH && (
{config.VITE_ENABLE_DOTACOACH && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also fixes a typo here.

<a
href="https://dota-coach.com?s=OpenDota&c=main"
target="_blank"
Expand Down Expand Up @@ -84,11 +85,7 @@ const Sponsors = ({ strings }) => (
</StyledDiv>
);

Sponsors.propTypes = {
strings: PropTypes.shape({}),
};

const mapStateToProps = (state) => ({
const mapStateToProps = (state: any) => ({
strings: state.app.strings,
});

Expand Down
0 src/components/Home/Styled.jsx → src/components/Home/Styled.tsx
View file
Open in desktop
File renamed without changes.
11 changes: 4 additions & 7 deletions src/components/Home/Why.jsx → src/components/Home/Why.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react';
import styled from 'styled-components';
import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import { IconOpenSource, IconStatsBars, IconWand } from '../Icons';
import constants from '../constants';

import { HomePageProps } from './Home';

const StyledDiv = styled.div`
margin: 50px auto 0;
text-align: center;
Expand Down Expand Up @@ -50,7 +51,7 @@ const StyledDiv = styled.div`
}
`;

const Why = ({ strings }) => (
const Why = ({ strings }: HomePageProps) => (
<StyledDiv>
<div className="whyList">
<div className="whyElement">
Expand Down Expand Up @@ -84,11 +85,7 @@ const Why = ({ strings }) => (
</StyledDiv>
);

Why.propTypes = {
strings: PropTypes.shape({}),
};

const mapStateToProps = state => ({
const mapStateToProps = (state: any) => ({
strings: state.app.strings,
});

Expand Down
0 src/components/Home/index.js → src/components/Home/index.ts
View file
Open in desktop
File renamed without changes.