Skip to content

Conversation

@missionmike
Copy link
Collaborator

@missionmike missionmike commented Aug 15, 2025

Description

Reintegrate Theme Package

This PR re-integrates the theme package directly into the app. Previously, the theme package was extracted and hosted separately on npm so that developers could contribute to the front-facing theme styles without having access to the entire app.

However, now that the app is setup as source-available under the Amp'd Community License 1.0, I'm reintegrating the theme back into the app itself.

Add support for Company Description Field

Solution for #104

Type of Change

Feature

Checklist

Before submitting this PR, please make sure:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have read the CONTRIBUTING.md file

Screenshots (if applicable)

Edit company description section:

image

Note: Please ensure all tests pass and the code follows the project's coding standards before
submitting.


Important

Reintegrates theme package into the main app, adds company description field, and updates components and tests for theme and company features.

  • Theme Integration:
    • Reintegrates theme package into the main app, removing @ampdresume/theme from package.json.
    • Updates theme/index.ts to include ThemeDavids and ThemeDefault.
  • Company Description:
    • Adds description field to Company model in prisma/schema.prisma and migration.sql.
    • Updates GraphQL queries and mutations in addCompany.ts, updateCompany.ts, getCompanies.ts, and getExperience.ts to handle description.
    • Modifies CompanyForm.tsx, CompanyItem.tsx, and CompanyList.tsx to support description input and display.
  • Components and Tests:
    • Adds PDFView and ResumeView components with tests in PDFView.test.tsx and ResumeView.test.tsx.
    • Implements Skills, SkillsCloud, and SkillsExperience components with tests.
    • Introduces ThemeDavids and ThemeDefault components with corresponding tests.
    • Adds QRGenerator, CertificationsSection, and FeaturedProjects components with tests.
    • Updates MUIThemeProvider.tsx for theme management.
  • Miscellaneous:
    • Removes import order rule from .eslintrc.json.
    • Adds sample data in davids/sampleData.json.

This description was created by Ellipsis for f287978. You can customize this summary. It will automatically update as commits are pushed.

@missionmike missionmike self-assigned this Aug 15, 2025
@missionmike missionmike added the enhancement New feature or request label Aug 15, 2025
@vercel
Copy link

vercel bot commented Aug 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
ampdresume Ready Ready Preview Comment Aug 15, 2025 4:01pm

@vercel
Copy link

vercel bot commented Aug 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
ampdresume Ready Ready Preview Comment Aug 15, 2025 3:19pm

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to 56595db in 6 minutes and 46 seconds. Click for details.
  • Reviewed 17384 lines of code in 357 files
  • Skipped 2 files when reviewing.
  • Skipped posting 22 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/graphql/server/resolvers/mutationDefs.ts:1
  • Draft comment:
    Consider using a server-side gql tag (e.g. from apollo-server) rather than '@apollo/client' in resolvers.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. src/graphql/server/resolvers/mutations/addCompany.ts:25
  • Draft comment:
    Validate date string format before appending '-02' to avoid unexpected results.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. src/app/about/privacy-policy/__snapshots__/page.test.tsx.snap:229
  • Draft comment:
    There appears to be a trailing space at the end of the line ("...contact us at "). Please remove this extra whitespace.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% Snapshot files are automatically generated by tests and reflect the exact output of components. The trailing space was intentionally removed in the diff. Commenting on whitespace in snapshot files is not useful since these files are meant to exactly match component output. The whitespace change was intentional as shown by the diff. Maybe the trailing space could affect visual rendering or accessibility of the component in some way? The space is between text and a link element - its presence or absence won't meaningfully impact rendering or accessibility. The author clearly intended to remove it. Delete this comment. Nitpicking whitespace in auto-generated snapshot files is not useful, and the change was clearly intentional.
4. src/app/about/terms-of-service/__snapshots__/page.test.tsx.snap:179
  • Draft comment:
    There appears to be an unintended extra whitespace line inserted here. Is the extra whitespace on this line intentional for the snapshot, or could it be removed to match the expected formatting?
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The whitespace changes appear intentional and consistent throughout the file. This is a snapshot test file, so the whitespace is part of the expected output. The changes follow a pattern of adding space before tags. Since this is a UI change, we should assume the author intended these whitespace changes. The whitespace could affect text wrapping and spacing in the UI, so maybe it's worth questioning. Also, snapshot tests are meant to catch unintended changes. While true, we should assume UI changes are intentional per the rules. The consistent pattern suggests this was a deliberate formatting change. Delete the comment. The whitespace changes appear intentional and consistent, and we should not question UI/formatting changes per the review rules.
5. src/app/edit/components/RichTextEditor/plugins/ColorPlugin.tsx:55
  • Draft comment:
    Typo: 'LOW_PRIORIRTY' seems to be misspelled. Consider changing it to 'LOW_PRIORITY' if that's the intended name.
  • Reason this comment was not posted:
    Comment was on unchanged code.
6. src/app/edit/components/RichTextEditor/plugins/ToolbarPlugin.tsx:35
  • Draft comment:
    Typo: 'LOW_PRIORIRTY' appears to be a misspelling. Did you mean 'LOW_PRIORITY'?
  • Reason this comment was not posted:
    Comment was on unchanged code.
7. src/app/edit/components/RichTextEditor/plugins/ToolbarPlugin.tsx:121
  • Draft comment:
    Typo: 'LOW_PRIORIRTY' seems to be misspelled. Consider renaming it to 'LOW_PRIORITY'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
8. src/app/edit/components/RichTextEditor/plugins/ToolbarPlugin.tsx:132
  • Draft comment:
    Typo: 'LOW_PRIORIRTY' appears here. It likely should be 'LOW_PRIORITY'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
9. src/app/edit/components/RichTextEditor/plugins/ToolbarPlugin.tsx:143
  • Draft comment:
    Typo: 'LOW_PRIORIRTY' is misspelled. Consider updating to 'LOW_PRIORITY'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
10. src/app/edit/components/RichTextEditor/plugins/YouTubePlugin.tsx:16
  • Draft comment:
    There is an inconsistency in the naming of the YouTube component. The file is named 'YouTubePlugin.tsx' (with an uppercase "T"), but the exported function is 'YoutubePlugin' (with a lowercase "t"). Consider standardizing the capitalization for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
11. src/app/edit/components/RichTextEditor/plugins/YouTubePlugin.tsx:67
  • Draft comment:
    Typographical error: The placeholder text "Add Youtube URL" is inconsistent with the proper branding 'YouTube' shown elsewhere (e.g., in the title "Embed YouTube Video"). Consider updating "Add Youtube URL" to "Add YouTube URL" for consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.
12. src/app/edit/components/RichTextEditor/useKeyBindings.tsx:35
  • Draft comment:
    Typographical error: 'LOW_PRIORIRTY' appears to be misspelled. Consider using 'LOW_PRIORITY'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
13. src/app/edit/profile/__snapshots__/page.test.tsx.snap:182
  • Draft comment:
    Typo: There appears to be a trailing whitespace at the end of the line. Please remove the extra space after "will".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a snapshot test file which is auto-generated based on component rendering. The whitespace change appears intentional as part of updating the snapshot. Whitespace in snapshots is not a code quality issue - it just needs to match the actual component output. Making the suggested change would make the snapshot not match reality. Maybe the whitespace actually indicates a bug in the component's rendering that should be fixed? Even if the whitespace was unintentional in the component, that should be addressed in the component code itself, not by manually editing the snapshot file. Snapshots should match actual output. Delete this comment. Snapshot files should match actual component output, not be manually edited for whitespace.
14. src/app/edit/profile/components/__snapshots__/AccountForm.test.tsx.snap:129
  • Draft comment:
    There seems to be a trailing space at the end of this line. Consider removing it for cleaner output.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a snapshot test file, which is auto-generated. Trailing spaces in snapshot files don't affect functionality. The space appears to be intentional as part of the snapshot update. Commenting on formatting in test snapshots is not useful since these files are meant to be exact matches of the rendered output. Maybe the trailing space could cause issues with snapshot comparison or affect the rendered output in some way? No - snapshot tests are meant to be exact matches of the actual output, including spaces. If the space was problematic, it would need to be fixed in the component rendering, not in the snapshot file. Delete this comment. Formatting suggestions on auto-generated snapshot files are not useful since they need to exactly match the rendered output.
15. src/app/page.tsx:229
  • Draft comment:
    Typographical issue: "cusomtized" appears to be a typo. It should be "customized".
  • Reason this comment was not posted:
    Comment was on unchanged code.
16. src/components/tooltips/__snapshots__/tooltips.test.tsx.snap:11
  • Draft comment:
    There's an extraneous whitespace-only line on line 11. If this blank line wasn't intentionally added as part of the snapshot, please remove it to match the expected output format.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a snapshot test file which captures the exact output format of a component. The whitespace appears to be intentional formatting. The comment suggests removing it, but if this is the actual rendered output we want to capture, removing it would make the snapshot inaccurate. Snapshot files are meant to capture exact output including whitespace. Maybe the whitespace is actually unintended and could cause visual inconsistencies in the rendered output? Since this is a UI component and the change is purely presentational, per the rules we should assume the author made the UI change correctly and intentionally. The comment should be deleted as it questions a UI formatting choice in a snapshot file, which we should assume was intentional.
17. src/theme/components/README.md:4
  • Draft comment:
    Typographical suggestion: The term 'common-ground' might be reconsidered; typically, it is written as 'common ground' without a hyphen, unless you intended an adjectival form. Please double-check if this was your intended usage.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 30% <= threshold 50% The comment suggests a typographical change and asks the author to double-check their intention. While it provides a suggestion, it also asks for confirmation of intent, which violates the rules.
18. src/theme/components/ResumeTitle/ResumeTitle.tsx:14
  • Draft comment:
    Typo: The media query string '@media screen and (max-width: $breakpoint_mobile)' uses $breakpoint_mobile, which might be a mistake. If you intended to use the theme's breakpoint (as done below with theme.breakpoints.down(\"sm\")), consider replacing it with the correct value or reference.
  • Reason this comment was not posted:
    Marked as duplicate.
19. src/theme/components/Skills/SkillsExperience.tsx:43
  • Draft comment:
    Typo note: The code uses yearStarted while the comment above references yearStart (e.g., line 20). Consider aligning the property name for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the comment is technically correct about the mismatch, this kind of documentation/implementation mismatch in example code is relatively minor. The actual property name yearStarted is used consistently in the implementation. The example in the comment is just illustrative and doesn't affect functionality. Comments about documentation usually don't warrant blocking a PR. The mismatch could potentially confuse future developers who are trying to understand the data structure. Documentation should ideally match implementation exactly. The example is clearly marked as illustrative, showing the general data structure transformation. The actual property name is used consistently in the implementation, which is what matters. While technically correct, this comment about documentation is too minor to be worth keeping. It doesn't affect functionality and the example is clearly illustrative.
20. src/theme/default/components/WorkExperience/Projects.tsx:6
  • Draft comment:
    Minor grammatical suggestion: consider rephrasing the comment text 'renders as list of all projects' to 'renders a list of all projects' for improved clarity.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 30% <= threshold 50% The comment is suggesting a minor grammatical change to improve clarity. While it is not asking for confirmation or testing, it is also not a code suggestion or related to functionality. It is more of a stylistic suggestion, which doesn't align with the rules for useful comments.
21. src/util/social.ts:27
  • Draft comment:
    Typo: The variable name socialPlatformdetails appears to be missing camel casing. Consider renaming it to socialPlatformDetails for consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.
22. src/util/structure.ts:38
  • Draft comment:
    Typographical error: The inline comment (line 15 in the doc comment) refers to yearStart, but the code uses yearStarted. Consider aligning these property names for clarity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While technically correct, this is a very minor documentation issue. The code itself works correctly, and the documentation still conveys the general structure and purpose. The mismatch doesn't affect functionality and is obvious enough that it won't confuse readers. Documentation nitpicks should generally be avoided unless they cause real confusion. The mismatch could potentially confuse someone trying to copy-paste from the example. Also, maintaining accurate documentation is important for long-term maintainability. The example is clearly marked as showing the data format conversion conceptually, not as exact code to copy. The property name difference is minor enough that it doesn't impact understanding the transformation. This comment should be deleted as it's too minor and doesn't affect functionality. It's the kind of nitpick that adds noise without providing significant value.

Workflow ID: wflow_LfJIgDOiJGhVyzdl

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

missionmike and others added 2 commits August 15, 2025 08:32
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed f287978 in 2 minutes and 31 seconds. Click for details.
  • Reviewed 29 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/theme/components/ResumeTitle/ResumeTitle.tsx:14
  • Draft comment:
    Good refactor using theme.breakpoints.down('sm') for responsive styles instead of a hardcoded media query.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. src/theme/default/components/WorkExperience/ProjectAccordion.tsx:72
  • Draft comment:
    Consistent use of theme.breakpoints.down('sm') for mobile overrides. Review if '!important' for padding is necessary.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_4i5sr3E1WlXVDSpT

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Skipped PR review on df43dde because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.

@missionmike missionmike merged commit c716727 into develop Aug 15, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants