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

Update Rewards Activities #1921

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Conversation

corlard3y
Copy link
Collaborator

@corlard3y corlard3y commented Oct 14, 2024

Pull Request Template

#1920

Description

  • Problem/Feature:
  • Add Cyber to Activities List

Type of Change

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe): Update Rewards Activities List

Checklist

  • Quick PR: Is this a quick PR? Can be approved before finishing a coffee.
    • Quick PR label added
  • Not Merge Ready: Is this PR dependent on some other PR/tasks and not ready to be merged right now.
    • DO NOT Merge PR label added

Frontend Guidelines

Build & Testing

  • No errors in the build terminal
  • Engineer has tested the changes on their local environment
  • Engineer has tested the changes on deploy preview

Screenshots/Video with Explanation

  • Before: Explain the previous behavior

  • After: What's changed now

Screenshot 2024-10-14 at 16 11 21

Additional Context

Review & Approvals

  • Self-review completed
  • Code review by at least one other engineer
  • Documentation updates if applicable

Notes

@corlard3y corlard3y linked an issue Oct 14, 2024 that may be closed by this pull request
Copy link

In the file src/modules/rewards/components/RewardsActivityIcon.tsx:

  • There are several places where components are not being returned correctly, e.g., missing 'return' keyword before the component declaration.
  • The 'if' conditions are missing the 'return' statements for the components that should be returned.
  • There are placeholder comments without actual code inside, e.g., '// ...'.
  • The 'if' conditions are inconsistent and should be switched to 'if-else if' or 'switch' statements for better clarity and efficiency.

In the file src/modules/rewards/components/RewardsActivityTitle.tsx:

  • In the else block, the return statement does not return the JSX element correctly. Need to wrap it in a JSX element like <></>.

Overall, the code seems to have issues with returning JSX elements correctly based on certain conditions and missing return statements in conditional checks.

Please address these issues to ensure the code functions correctly.

Copy link

github-actions bot commented Oct 14, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-10-15 11:54 UTC

Copy link
Collaborator

@rohitmalhotra1420 rohitmalhotra1420 left a comment

Choose a reason for hiding this comment

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

@corlard3y I cannot see these activity and changes on deploy preview. Please add a video of the env it's present or get it added on dev so I can test it out on deploy preview.

src/common/Common.baseLogos.ts Outdated Show resolved Hide resolved
src/modules/rewards/components/RewardsActivityIcon.tsx Outdated Show resolved Hide resolved
src/modules/rewards/components/RewardsActivityTitle.tsx Outdated Show resolved Hide resolved
@corlard3y
Copy link
Collaborator Author

@corlard3y I cannot see these activity and changes on deploy preview. Please add a video of the env it's present or get it added on dev so I can test it out on deploy preview.

I think this is only on Staging, already pinged Shoaib to deploy to dev as well.
Screenshot 2024-10-14 at 16 11 21

Copy link

All looks good.

@rohitmalhotra1420 rohitmalhotra1420 merged commit c586fd8 into main Oct 15, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

Update Rewards Activities
2 participants