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

Testing PR for lzay loading #1770

Closed
wants to merge 1 commit into from
Closed

Testing PR for lzay loading #1770

wants to merge 1 commit into from

Conversation

abhishek-01k
Copy link
Collaborator

Pull Request Template

Ticket Number

Description

  • Problem/Feature:

Type of Change

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe):

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

Additional Context

Review & Approvals

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

Notes

@abhishek-01k abhishek-01k added the DO NOT MERGE If the PR/task is dependent on some other task add this label to the PR. label Jul 29, 2024
Copy link

  • In the import statement, the components Add, Box, Dash, HoverableSVG, and Text are imported from 'blocks'. It is assumed that these components are available in the 'blocks' package. Please verify if the import path is correct.

  • The margin prop in the <Box> component has redundant values. Consider cleaning it up for better readability.

  • In the <Text> component, the variant prop is set to "h3-semibold". Ensure that this variant is defined and available if it's a custom variant.

  • The onClick handler inline function in the <Box> component looks correct. It toggles the visibility of the subheader by calling setSubHeaderVisibility with the opposite value of showSubHeader.

  • Check if the HoverableSVG component is handling the icon prop correctly and the icons (Dash and Add) are rendering appropriately.

  • Overall, the component structure and props usage seem fine. Make sure to thoroughly test the functionality of this component in the application.

All looks good.

Copy link

github-actions bot commented Jul 29, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-07-29 08:56 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE If the PR/task is dependent on some other task add this label to the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant