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

carbon capture modal #1979

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open

Conversation

sunilsabatp
Copy link
Contributor

image

Copy link

vercel bot commented Feb 16, 2024

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

Name Status Preview Comments Updated (UTC)
planet-webapp ✅ Ready (Inspect) Visit Preview Jul 6, 2024 8:16am
planet-webapp-multi-tenancy-setup ✅ Ready (Inspect) Visit Preview Jul 6, 2024 8:16am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
planet-webapp-temp ⬜️ Ignored (Inspect) Visit Preview Jul 6, 2024 8:16am

Copy link
Collaborator

@prachigarg19 prachigarg19 left a comment

Choose a reason for hiding this comment

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

@sunilsabatp left some feedback.

<div className={style.carbonCapture}>
{t('projectDetails:totalCO₂Captured')}
</div>
<div className={style.unit}>(tons)</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to translate tons as well. There is already a key present for this.

src/temp/CarbonCapture/CarbonCapture.tsx Outdated Show resolved Hide resolved
<InfoIcon />
</div>
</div>
<div className={style.carbonCatureDetailContainer}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why have you provided static width here, won't this be dependent on the values received from api?

src/temp/CarbonCapture/CarbonCapture.tsx Outdated Show resolved Hide resolved
src/temp/stories/CarbonCapture.stories.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

coderabbitai bot commented Feb 28, 2024

Walkthrough

The changes introduce several new components and styles related to carbon capture visualization in a React application. A BarGraph component displays carbon capture metrics, while CarbonCapture provides a tabbed interface for viewing data. Custom styles are defined in a new SCSS file, and a new icon component is modified. Additionally, Storybook stories are created for the CarbonCapture component to facilitate testing and documentation.

Changes

File Change Summary
src/temp/CarbonCapture/BarGraph.tsx Added CO2BarGraph and CO2CaptureData components; introduced props and a helper function.
src/temp/CarbonCapture/CarbonCapture.module.scss Created styles for the Carbon Capture component using flexbox and gradients.
src/temp/CarbonCapture/CustomMuiTabs.tsx Added CustomMuiTabs styled component based on Material-UI Tabs with custom styles.
src/temp/CarbonCapture/index.tsx Introduced CarbonCapture component with tab functionality; added internal state management.
src/temp/icons/NewInfoIcon.tsx Updated NewInfoIcon component by removing the height property from props and SVG element.
src/temp/stories/CarbonCapture.stories.tsx Created Storybook stories for CarbonCapture with two variations: SiteSelected and EntireProjectSelected.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CarbonCapture
    participant CustomMuiTabs
    participant CO2BarGraph
    participant CO2CaptureData

    User->>CarbonCapture: Select tab
    CarbonCapture->>CustomMuiTabs: Update selected tab
    CustomMuiTabs->>CarbonCapture: Render selected tab content
    CarbonCapture->>CO2BarGraph: Render CO2BarGraph with props
    CarbonCapture->>CO2CaptureData: Render CO2CaptureData with props
Loading

🐇 "In the land of carbon dreams,
New graphs and tabs burst at the seams.
With colors bright and data clear,
We capture CO₂ without a fear.
Hop along, let’s visualize,
Together we’ll reach for the skies!" 🌍✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

prachigarg19

This comment was marked as resolved.

src/temp/CarbonCapture/BarGraph.tsx Outdated Show resolved Hide resolved
public/static/locales/en/projectDetails.json Outdated Show resolved Hide resolved
src/temp/CarbonCapture/CarbonCapture.tsx Outdated Show resolved Hide resolved
<CO2CaptureData {...carbonCaptureDataProps} />
</div>
</div>
<button
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've added a note that this needs to be replaced with the common component created in AboutProject PR

src/temp/CarbonCapture/CarbonCapture.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@prachigarg19 prachigarg19 left a comment

Choose a reason for hiding this comment

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

@sunilsabatp kindly look into the feedback

Copy link
Collaborator

@prachigarg19 prachigarg19 left a comment

Choose a reason for hiding this comment

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

LGTM

@mohitb35 mohitb35 force-pushed the feature/redesign-explore-btn branch 3 times, most recently from f54228e to 9ea7417 Compare April 11, 2024 12:00
@mohitb35 mohitb35 added the blocked Should not be merged label Apr 17, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Outside diff range and nitpick comments (6)
src/temp/CarbonCapture/CarbonCapture.module.scss (4)

9-38: LGTM: Well-structured info container and label styles.

The styles for the info container and labels are well-organized and use flexbox effectively. However, consider using CSS custom properties (variables) for repeated values like font sizes and gaps to improve maintainability.

For example, you could define:

:root {
  --font-size-xxsmall: $fontXXSmall;
  --gap-small: 12px;
}

And then use them like:

.carbonCaptureInfoContainer {
  gap: var(--gap-small);
}

.carbonCaptureLabelMainContainer {
  font-size: var(--font-size-xxsmall);
}

86-136: LGTM: Well-organized data container and text styles.

The styles for data containers and text elements are well-structured and provide a clear visual hierarchy. However, there are opportunities for improvement:

  1. Consider using CSS custom properties for repeated values like font sizes and weights.
  2. The use of absolute units (px) for font sizes may limit accessibility. Consider using relative units like rem or em.

Here's an example of how you could refactor this:

:root {
  --font-size-xxxsmall: 0.75rem; // Equivalent to 12px at default browser settings
  --font-weight-normal: 400;
  --font-weight-bold: 700;
}

.beforeIntervationData,
.byProjectData,
.sitePotentialData {
  font-size: var(--font-size-xxxsmall);
  font-weight: var(--font-weight-bold);
}

.beforeIntervationLabel,
.byProjectLabel,
.sitePotentialLabel {
  font-size: var(--font-size-xxxsmall);
  font-weight: var(--font-weight-normal);
}

This approach will make the styles more maintainable and accessible.


149-170: LGTM: Visually appealing "See More" button styles with room for improvement.

The "See More" button styles are visually appealing with the use of a linear gradient. However, there are opportunities for improvement:

  1. Consider using relative units (e.g., em, rem) for dimensions and font sizes to improve responsiveness and accessibility.
  2. The fixed height of the button might cause issues with different font sizes or longer text. Consider using padding instead.

Here's an example of how you could refactor this:

.seeMoreButton {
  background: linear-gradient(0deg, $primaryColorNew 0%, $primaryColorNew 100%), $light;
  width: 100%;
  padding: 0.5em 1em;
  margin-top: 1.5rem;
  display: flex;
  justify-content: center;
  align-items: center;
  cursor: pointer;
}

.seeMoreLabel {
  font-size: 0.75rem;
  font-weight: 700;
  color: $light;
}

This approach will make the button more flexible and accessible.


172-185: LGTM: Clear miscellaneous styles with a minor concern.

The miscellaneous styles are clear and serve their purposes well. However, the use of relative positioning for the horizontal line might cause layout issues in some scenarios.

Consider using margin or padding to adjust the position of the horizontal line instead of relative positioning. This approach is generally more predictable and less likely to cause unexpected layout shifts. For example:

.horizontalLine {
  margin-top: -8px;
}
src/temp/CarbonCapture/index.tsx (2)

33-33: Rename 'isExtend' to 'isExpanded' for clarity.

The state variable isExtend may be better named isExpanded to more accurately reflect its purpose and improve code readability.

Apply this diff to rename the variable:

- const [isExtend, setIsExtend] = useState(false);
+ const [isExpanded, setIsExpanded] = useState(false);

And update all references accordingly:

- onClick={() => setIsExtend(!isExtend)}
+ onClick={() => setIsExpanded(!isExpanded)}

- {isExtend ? t('seeLess') : t('seeMore')}
+ {isExpanded ? t('seeLess') : t('seeMore')}

- {isExtend ? <UpArrow width={7} /> : <DownArrow width={7} />}
+ {isExpanded ? <UpArrow width={7} /> : <DownArrow width={7} />}

59-61: Provide an accessible label for 'NewInfoIcon' to enhance accessibility.

To improve accessibility, consider adding an aria-label to the NewInfoIcon so that screen readers can convey its purpose to users.

Apply this diff:

 <div className={style.infoIconContainer}>
-  <NewInfoIcon width={17.6} color={light.dividerColorNew} />
+  <NewInfoIcon
+    width={17.6}
+    color={light.dividerColorNew}
+    aria-label={t('carbonCaptureInfo')}
+  />
 </div>

Ensure that 'carbonCaptureInfo' is defined in your translations as appropriate.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 18ffaaf and ad0d272.

⛔ Files ignored due to path filters (2)
  • .github/workflows/chromatic.yml is excluded by !**/*.yml
  • public/static/locales/en/projectDetails.json is excluded by !**/*.json
📒 Files selected for processing (6)
  • src/temp/CarbonCapture/BarGraph.tsx (1 hunks)
  • src/temp/CarbonCapture/CarbonCapture.module.scss (1 hunks)
  • src/temp/CarbonCapture/CustomMuiTabs.tsx (1 hunks)
  • src/temp/CarbonCapture/index.tsx (1 hunks)
  • src/temp/icons/NewInfoIcon.tsx (1 hunks)
  • src/temp/stories/CarbonCapture.stories.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/temp/stories/CarbonCapture.stories.tsx
🧰 Additional context used
🔇 Additional comments (8)
src/temp/CarbonCapture/CustomMuiTabs.tsx (3)

1-1: LGTM: Imports are correct and concise.

The necessary imports from '@mui/material' are present for creating a styled Tabs component.


33-33: LGTM: Export statement is correct.

The default export of the CustomMuiTabs component is appropriate and follows common practices for React components.


1-33: Verify integration with the CarbonCapture component and consider documentation.

The CustomMuiTabs component appears well-structured for its purpose in the carbon capture visualization feature. To ensure its proper integration:

  1. Verify that this component is correctly used within the CarbonCapture component (src/temp/CarbonCapture/index.tsx).
  2. Consider adding JSDoc comments to describe the component's purpose and any props it might accept in the future.
  3. Ensure that the CSS variables used (--primary-font-color, --primary-color-new) are defined in your global styles or theme provider.

To check the integration, run:

✅ Verification successful

Integration Verified Successfully

The CustomMuiTabs component is correctly integrated within the CarbonCapture component, and the required CSS variables are properly defined in the global styles.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify CustomMuiTabs usage in CarbonCapture component

# Test: Search for CustomMuiTabs import and usage
rg -A 5 "CustomMuiTabs" src/temp/CarbonCapture/index.tsx

# Test: Verify CSS variables are defined
rg --type css "(--primary-font-color|--primary-color-new)"

Length of output: 834

src/temp/CarbonCapture/BarGraph.tsx (2)

1-9: LGTM: Imports and interface declaration are well-structured.

The imports are appropriate for the components' needs, and the CO2BarGraphProps interface is correctly defined with the required properties.


54-101: ⚠️ Potential issue

Add error handling for potential issues.

The component doesn't currently handle potential errors or edge cases. Consider adding checks for undefined or invalid input values to improve robustness.

To verify the need for error handling, let's check if there are any existing error checks or if the parent component handles this:

If the script doesn't find any error handling, consider adding checks like this:

if (isNaN(beforeIntervation) || isNaN(byProject) || isNaN(sitePotential)) {
  return <div>Error: Invalid input data</div>;
}

This will prevent rendering with invalid data and improve the user experience.

src/temp/CarbonCapture/CarbonCapture.module.scss (3)

1-7: LGTM: Proper theme import and main container setup.

The theme import and main container styles are well-structured. Using flexbox for the main container layout is a good choice for responsive design.


138-147: LGTM: Clear modal and info icon styles.

The styles for the modal and info icon are concise and appropriate. The use of max-width for the modal is a good practice for responsive design.


40-84: LGTM: Visually appealing detail container and indicator styles.

The detail container and indicator styles are well-designed, using linear gradients and specific colors for visual appeal. However, ensure that the color contrast ratios meet WCAG 2.1 Level AA standards for accessibility.

To verify the color contrast, you can use a tool like the WebAIM Color Contrast Checker. Run the following script to output the background and text colors used:

Please check these color combinations against the WCAG 2.1 Level AA standards.

Comment on lines +3 to +31
const CustomMuiTabs = styled(Tabs)(() => ({
'.MuiTabs-flexContainer': {
justifyContent: 'center',
marginTop: '24px',
gap: '30px',
},
'.MuiButtonBase-root': {
textTransform: 'none',
fontSize: '12px',
color: `var(--primary-font-color)`,
},
'.MuiButtonBase-root-MuiTab-root.Mui-selected': {
color: `var(--primary-color-new)`,
},
'.MuiTabs-indicator': {
backgroundColor: `var(--primary-color-new)`,
},
'.MuiTab-root.Mui-selected': {
color: `var(--primary-color-new)`,
display: 'flex',
flexDirection: 'row',
},
'.MuiTab-root': {
minHeight: '0px',
padding: '0px 0px',
display: 'flex',
flexDirection: 'row',
},
}));
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider enhancing responsiveness and reusability of the CustomMuiTabs component.

The styled component effectively customizes the Material-UI Tabs with specific styles. However, there are a few suggestions for improvement:

  1. Replace pixel values with relative units (e.g., rem or em) for better responsiveness. For example:

    - fontSize: '12px',
    + fontSize: '0.75rem',
  2. Consider making some styles configurable through props for better reusability. For instance:

    const CustomMuiTabs = styled(Tabs)<{ gap?: string }>(({ gap = '30px' }) => ({
      // ...
      '.MuiTabs-flexContainer': {
        gap: gap,
        // ...
      },
      // ...
    }));
  3. Ensure that the CSS variables (e.g., --primary-font-color, --primary-color-new) are defined in your global styles or theme provider.

Would you like assistance in implementing these suggestions?

@@ -1,12 +1,11 @@
import React from 'react';
import { IconProps } from '../../features/common/types/common';

const NewInfoIcon = ({ height, width, color }: IconProps) => {
const NewInfoIcon = ({ width, color }: IconProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

Issues Found: Inconsistent Removal of height Prop

  • The following components still pass the height prop to NewInfoIcon:
    • src/temp/components/InfoIconPopup.tsx uses <NewInfoIcon height={height} width={width} color={color} />
    • src/temp/ProjectBadge/ProjectBadge.tsx uses <NewInfoIcon width={10} height={10} color={'var(--light)'} />
  • Other SVG icons like ExpandIcon.tsx still utilize the height attribute, causing inconsistency across icon components.
🔗 Analysis chain

Verify impact of removing height prop and suggest improvements

The removal of the height prop from the component signature simplifies the interface, but it may have some implications:

  1. Ensure this change is consistent with other icon components in the project.
  2. Verify that removing the height attribute from the SVG doesn't negatively impact the icon's rendering or aspect ratio.
  3. Check usage of this component throughout the project and update any instances passing a height prop.
  4. Consider adding aria-hidden="true" to the SVG for better accessibility, as this appears to be a decorative icon.

To ensure consistency and identify potential issues, run the following script:

Consider adding aria-hidden="true" to improve accessibility:

 const NewInfoIcon = ({ width, color }: IconProps) => {
   return (
     <svg
+      aria-hidden="true"
       xmlns="http://www.w3.org/2000/svg"
       width={width}
       viewBox="0 0 10 10"
       fill="none"
     >
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistency in icon components and usage of NewInfoIcon

# Test 1: Check other icon components for consistency
echo "Checking other icon components for height prop usage:"
ast-grep --pattern 'const $_ = ({ height, $$$}: IconProps) => {' src/temp/icons

# Test 2: Find usage of NewInfoIcon component
echo "Checking usage of NewInfoIcon component:"
rg "NewInfoIcon" -A 5 src

# Test 3: Check if other SVG icons use height attribute
echo "Checking if other SVG icons use height attribute:"
rg '<svg.*height=' src/temp/icons

Length of output: 3795

Comment on lines +1 to +101
import { useTranslations, useLocale } from 'next-intl';
import { getFormattedNumber } from '../../utils/getFormattedNumber';

interface CO2BarGraphProps {
beforeIntervation: number;
byProject: number;
sitePotential: number;
}

export const CO2BarGraph = ({
beforeIntervation,
byProject,
sitePotential,
}: CO2BarGraphProps) => {
const calculatePercentage = (value: number, sitePotential: number) => {
return (value / sitePotential) * 100;
};

const beforeIntervationPercentage = calculatePercentage(
beforeIntervation,
sitePotential
);
const byProjectPercentage = calculatePercentage(byProject, sitePotential);

return (
<div className={style.carbonCaptureIndicator}>
<div
style={{
width: `${beforeIntervationPercentage}%`,
}}
className={style.beforeIntervationIndicator}
/>

<div
style={{
width: `${byProjectPercentage}%`,
}}
className={style.byProjectIndicator}
/>

<div
style={{
width: `${
100 - (beforeIntervationPercentage + byProjectPercentage)
}%`,
}}
className={style.projectPotential}
/>
</div>
);
};

export const CO2CaptureData = ({
beforeIntervation,
byProject,
sitePotential,
}: CO2BarGraphProps) => {
const t = useTranslations('ProjectDetails');
const locale = useLocale();
return (
<div className={style.carbonCaptureDataContainerMain}>
<div>
<p className={style.beforeIntervationData}>
{t('co₂Quantity', {
quantity: getFormattedNumber(locale, beforeIntervation),
})}
</p>
<p className={style.beforeIntervationLabel}>
{t('beforeIntervention')}
</p>
<p className={style.beforeIntervationDate}>
{t('before', {
date: 2018,
})}
</p>
</div>
<div>
<p className={style.byProjectData}>
{t('byProjectCO₂Quantity', {
quantity: getFormattedNumber(locale, byProject),
})}
</p>
<p className={style.byProjectLabel}>{t('byProject')}</p>
<p className={style.byProjectDate}>
{t('since', {
date: 2018,
})}
</p>
</div>
<div className={style.sitePotentialDataContainer}>
<p className={style.sitePotentialData}>
{t('co₂Quantity', {
quantity: getFormattedNumber(locale, sitePotential),
})}
</p>
<p className={style.sitePotentialLabel}>{t('sitePotential')}</p>
</div>
</div>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider overall structural and type improvements.

  1. The CO2BarGraph and CO2CaptureData components are closely related. Consider combining them into a single component or creating a wrapper component that manages both, to reduce prop drilling and improve cohesion.

  2. Enhance type safety by creating a more specific type for the formatted numbers:

type FormattedNumber = string;

interface CO2CaptureProps {
  beforeIntervation: number;
  byProject: number;
  sitePotential: number;
  getFormattedNumber: (locale: string, value: number) => FormattedNumber;
}

This change will make the component more robust and easier to maintain in the long run.

Comment on lines +11 to +52
export const CO2BarGraph = ({
beforeIntervation,
byProject,
sitePotential,
}: CO2BarGraphProps) => {
const calculatePercentage = (value: number, sitePotential: number) => {
return (value / sitePotential) * 100;
};

const beforeIntervationPercentage = calculatePercentage(
beforeIntervation,
sitePotential
);
const byProjectPercentage = calculatePercentage(byProject, sitePotential);

return (
<div className={style.carbonCaptureIndicator}>
<div
style={{
width: `${beforeIntervationPercentage}%`,
}}
className={style.beforeIntervationIndicator}
/>

<div
style={{
width: `${byProjectPercentage}%`,
}}
className={style.byProjectIndicator}
/>

<div
style={{
width: `${
100 - (beforeIntervationPercentage + byProjectPercentage)
}%`,
}}
className={style.projectPotential}
/>
</div>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

LGTM with a minor optimization suggestion.

The CO2BarGraph component is well-implemented and correctly visualizes the carbon capture data. Consider this small optimization:

You can optimize the percentage calculations by computing them once and reusing the results:

-  const beforeIntervationPercentage = calculatePercentage(
-    beforeIntervation,
-    sitePotential
-  );
-  const byProjectPercentage = calculatePercentage(byProject, sitePotential);
+  const beforeIntervationPercentage = calculatePercentage(beforeIntervation, sitePotential);
+  const byProjectPercentage = calculatePercentage(byProject, sitePotential);
+  const remainingPercentage = 100 - (beforeIntervationPercentage + byProjectPercentage);

   // ... in the JSX
-          width: `${
-            100 - (beforeIntervationPercentage + byProjectPercentage)
-          }%`,
+          width: `${remainingPercentage}%`,

This change improves readability and avoids recalculating the remaining percentage.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const CO2BarGraph = ({
beforeIntervation,
byProject,
sitePotential,
}: CO2BarGraphProps) => {
const calculatePercentage = (value: number, sitePotential: number) => {
return (value / sitePotential) * 100;
};
const beforeIntervationPercentage = calculatePercentage(
beforeIntervation,
sitePotential
);
const byProjectPercentage = calculatePercentage(byProject, sitePotential);
return (
<div className={style.carbonCaptureIndicator}>
<div
style={{
width: `${beforeIntervationPercentage}%`,
}}
className={style.beforeIntervationIndicator}
/>
<div
style={{
width: `${byProjectPercentage}%`,
}}
className={style.byProjectIndicator}
/>
<div
style={{
width: `${
100 - (beforeIntervationPercentage + byProjectPercentage)
}%`,
}}
className={style.projectPotential}
/>
</div>
);
};
export const CO2BarGraph = ({
beforeIntervation,
byProject,
sitePotential,
}: CO2BarGraphProps) => {
const calculatePercentage = (value: number, sitePotential: number) => {
return (value / sitePotential) * 100;
};
const beforeIntervationPercentage = calculatePercentage(beforeIntervation, sitePotential);
const byProjectPercentage = calculatePercentage(byProject, sitePotential);
const remainingPercentage = 100 - (beforeIntervationPercentage + byProjectPercentage);
return (
<div className={style.carbonCaptureIndicator}>
<div
style={{
width: `${beforeIntervationPercentage}%`,
}}
className={style.beforeIntervationIndicator}
/>
<div
style={{
width: `${byProjectPercentage}%`,
}}
className={style.byProjectIndicator}
/>
<div
style={{
width: `${remainingPercentage}%`,
}}
className={style.projectPotential}
/>
</div>
);
};

Comment on lines +54 to +101
export const CO2CaptureData = ({
beforeIntervation,
byProject,
sitePotential,
}: CO2BarGraphProps) => {
const t = useTranslations('ProjectDetails');
const locale = useLocale();
return (
<div className={style.carbonCaptureDataContainerMain}>
<div>
<p className={style.beforeIntervationData}>
{t('co₂Quantity', {
quantity: getFormattedNumber(locale, beforeIntervation),
})}
</p>
<p className={style.beforeIntervationLabel}>
{t('beforeIntervention')}
</p>
<p className={style.beforeIntervationDate}>
{t('before', {
date: 2018,
})}
</p>
</div>
<div>
<p className={style.byProjectData}>
{t('byProjectCO₂Quantity', {
quantity: getFormattedNumber(locale, byProject),
})}
</p>
<p className={style.byProjectLabel}>{t('byProject')}</p>
<p className={style.byProjectDate}>
{t('since', {
date: 2018,
})}
</p>
</div>
<div className={style.sitePotentialDataContainer}>
<p className={style.sitePotentialData}>
{t('co₂Quantity', {
quantity: getFormattedNumber(locale, sitePotential),
})}
</p>
<p className={style.sitePotentialLabel}>{t('sitePotential')}</p>
</div>
</div>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider extracting the hardcoded year to a constant or prop.

The year 2018 is used multiple times in the component. To improve maintainability and flexibility, consider extracting it to a constant or passing it as a prop.

+ const INTERVENTION_YEAR = 2018;
// or add it to the component props

// Then use it in the component
-            date: 2018,
+            date: INTERVENTION_YEAR,

This change will make it easier to update the year in the future if needed.

Committable suggestion was skipped due to low confidence.

Comment on lines +68 to +71
<button
className={style.seeMoreButton}
onClick={() => setIsExtend(!isExtend)}
>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Specify the button type to prevent unintended form submissions.

To prevent any unintended form submission behavior, specify the button's type attribute as "button".

Apply this diff:

 <button
   className={style.seeMoreButton}
+  type="button"
   onClick={() => setIsExpanded(!isExpanded)}
 >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button
className={style.seeMoreButton}
onClick={() => setIsExtend(!isExtend)}
>
<button
className={style.seeMoreButton}
type="button"
onClick={() => setIsExtend(!isExtend)}
>

interface CarbonCaptureProps {
index: number;
value: number;
beforeIntervation: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the spelling of 'beforeIntervation' to 'beforeIntervention'.

The property beforeIntervation is misspelled. Please correct it to beforeIntervention throughout the code to maintain consistency and avoid confusion.

Apply this diff to fix the spelling:

 interface CarbonCaptureProps {
   index: number;
-  beforeIntervation: number;
+  beforeIntervention: number;
   byProject: number;
   sitePotential: number;
   area: number;
 }

And update all occurrences in the code.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
beforeIntervation: number;
beforeIntervention: number;


interface CarbonCaptureProps {
index: number;
value: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove the unused 'value' property from 'CarbonCaptureProps'.

The value property in CarbonCaptureProps is not used in the CarbonCapture component. Consider removing it to simplify the interface.

Apply this diff to remove value from the interface:

 interface CarbonCaptureProps {
   index: number;
-  value: number;
   beforeIntervention: number;
   byProject: number;
   sitePotential: number;
   area: number;
 }

And adjust the TabPanelProps type if necessary:

-type TabPanelProps = Omit<CarbonCaptureProps, 'area'>;
+type TabPanelProps = Omit<CarbonCaptureProps, 'area' | 'value'>;

Committable suggestion was skipped due to low confidence.

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.

5 participants