Skip to content

Release November 2nd, 2022#1597

Merged
mariahosfeld merged 46 commits into
mainfrom
develop
Nov 2, 2022
Merged

Release November 2nd, 2022#1597
mariahosfeld merged 46 commits into
mainfrom
develop

Conversation

@mariahosfeld
Copy link
Copy Markdown
Contributor

@mariahosfeld mariahosfeld commented Oct 26, 2022

Includes:
Features
#1579 Feature/change route plant location
#1592 Feature/add giftfund

Other
#1585 Dashboard maintenace refactor
#1594 add notion link
#1598 Language Update
#1601 Release cleanup

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 26, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
planet-webapp ✅ Ready (Inspect) Visit Preview Nov 2, 2022 at 2:19PM (UTC)

mohitb35 and others added 2 commits October 27, 2022 16:49
- along with 77126b5
- uses a common InlineFormDisplayGroup where needed
- fix alignment in groups not containing only text fields with type prop
- update min text field width to 180px
mohitb35 and others added 7 commits October 31, 2022 11:33
- reason > lighter API call
- also update types/interfaces
a. rename `APISingleProject` > `MapSingleProject` to reflect API resp.
b. use `MapSingleProject` instead of `SingleProject` in DonationLinkForm
c. remove unused APISingleProject/SingleProject interfaces
d. move `MapSingleProject` to project type file
@sagararyal
Copy link
Copy Markdown
Member

sagararyal commented Oct 31, 2022

…Route_plantLocation

Feature/change route plant location
@mariahosfeld mariahosfeld changed the title Release November 2022 Release November 2nd, 2022 Nov 2, 2022
@mohitb35 mohitb35 self-requested a review November 2, 2022 10:48
Comment thread pages/[p].tsx Outdated
import { getAllPlantLocations } from '../src/utils/maps/plantLocations';
import i18next from '../i18n';
import { SingleProjectGeojson } from '../src/features/common/types/project';
import { Treemapper } from '../src/features/user/TreeMapper/Treemapper';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is not necessary to import a namespace. The namespace Treemapper can be referenced within the application without importing it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Addressed here - #1601

@@ -1,4 +1,4 @@
declare namespace Treemapper {
export declare namespace Treemapper {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You do not need to export a namespace. Once declared it is accessible throughout the application.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Addressed here - #1601

Comment thread pages/[p].tsx Outdated
@@ -28,11 +29,15 @@ export default function Donate({
const [internalCurrencyCode, setInternalCurrencyCode] = React.useState('');
const [internalLanguage, setInternalLanguage] = React.useState('');
const [open, setOpen] = React.useState(false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

open is not an unused variable, so we can precede it with _ to remove the TS warning.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Addressed here - #1601

: '';

const { selectedPl, hoveredPl } = React.useContext(ProjectPropsContext);
const { selectedPl, hoveredPl, selectedSite } =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't seem to be using selectedSite here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Addressed here - #1601

<Head>
<title>{t('giftFund')}</title>
</Head>
<GiftFunds />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps this TS warning should be addressed.

  Its return type 'false | Element' is not a valid JSX element.
    Type 'boolean' is not assignable to type 'ReactElement<any, any>'.```

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Addressed here - #1601

Comment on lines +15 to +39
{user.planetCash.giftFunds
.filter((gift) => gift.openUnits !== 0)
.map((gift, index) => (
//Not displaying details for gift fund where open units = 0
<div className={styles.container} key={index}>
<div className={styles.container_heading}>
<b>
{user.planetCash?.country}/{user.planetCash?.currency}{' '}
{t('title')}
</b>
</div>
<hr />
<div className={styles.container_details}>
<div className={styles.project}>
<b>{t('project')}</b>
<p>{gift.project}</p>
</div>

<div className={styles.unit}>
<b>{t('units')}</b>
<p>{Number(gift.openUnits / 100).toFixed(2)}</p>
</div>
</div>
</div>
))}
Copy link
Copy Markdown
Member

@mohitb35 mohitb35 Nov 2, 2022

Choose a reason for hiding this comment

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

It may make sense to clean this up by separating the render logic into a different function. Also, address the TS errors in this file. - Noted in #1600

Comment on lines +23 to +38
ready && (
<DashboardView
title={t('title')}
subtitle={
<p>
{t('description1')}
<br />
{t('description2')}
</p>
}
>
<Details />
</DashboardView>
)
);
};
Copy link
Copy Markdown
Member

@mohitb35 mohitb35 Nov 2, 2022

Choose a reason for hiding this comment

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

It may make sense to refactor this at a later time to use common components like SingleColumnView - Noted in #1600

hr {
border: 1px solid #DDDBDA;
}
}
Copy link
Copy Markdown
Member

@mohitb35 mohitb35 Nov 2, 2022

Choose a reason for hiding this comment

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

It may be good to refactor this slightly to use styled css in a similar manner to the other features added to the dashboard recently. (At a later date) - Noted in #1600

mohitb35 and others added 4 commits November 2, 2022 19:09
> as namespaces do not need to exported/imported
- due to unused state variable `open`
- unused context parameter `selectedSite`
- explicitly define return types for functional component `Details`
- component was implicitly returning `JSX.Element` or `false`
- refactor code to return `ReactElement` | `null`
Copy link
Copy Markdown
Contributor

@Shreyaschorge Shreyaschorge left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants