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

Changed featured list #1901

Merged
merged 2 commits into from
Oct 7, 2024
Merged

Changed featured list #1901

merged 2 commits into from
Oct 7, 2024

Conversation

HarshRajat
Copy link
Contributor

Pull Request Template

Ticket Number

#1896

  • Problem/Feature: Updated Feature List

Type of Change

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe): Updated Channel Feature 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

Additional Context

Review & Approvals

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

Notes

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 5, 2024

  • In the environment variables section, there is a typo in VITE_APP_DEPLOY_ENV=DEV it should be VITE_APP_DEPLOY_ENV=DEV without the extra 'N' at the end.
  • There is an HTML declaration <!DOCTYPE html> that is misplaced. It should be at the very beginning of an HTML document.
  • There's no closing </script> tag at the end of the script under the </script> module tag. It should be added to close the script element.
  • In the script section, the import statement should not be there as it is unrelated to the HTML structure. It should be moved to a separate TypeScript file.
  • In the TypeScript code section, there is a reference to appConfig that is not defined within the provided section. It should be imported from its appropriate source or defined within the same file.

All looks good.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 5, 2024 •
edited
Loading

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-10-07 05:34 UTC

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 5, 2024

In the .env.sample file, there seems to be a typo in the comment for VITE_APP_NODE_ENV. It mentions DEPLOY_ENN instead of DEPLOY_ENV. Additionally, there's a duplicate content attribute in the meta tag for theme-color, and the comment for PUBLIC_URL is missing its closing tag.

In the index.html file, there are missing closing tags for the meta and link elements, multiple instances of name attribute without meta tag, and a missing closing tag for the meta tag related to theme-color.

In the public/index-dev.html file, the same issues present in index.html can be found, including missing closing tags and misplaced attributes.

For the other files not provided, it's recommended to verify that the structure and tags are correctly closed and nested as required.

Overall, there are several HTML syntax issues in the files provided that need to be fixed.

@HarshRajat HarshRajat changed the title changed featured list Changed featured list Oct 5, 2024
@rohitmalhotra1420 rohitmalhotra1420 merged commit f0aebc2 into main Oct 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rohitmalhotra1420 rohitmalhotra1420 rohitmalhotra1420 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@HarshRajat @rohitmalhotra1420