Migrate theme onepage-plus #59
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@alexanderdavide is attempting to deploy a commit to the JSON Resume Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes involve updating a project to include a new resume theme called Changes
Related issues
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 X ? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 10
Configuration used: CodeRabbit UI
Files ignored due to filter (6)
- apps/registry/package.json
- packages/jsonresume-theme-onepage-plus/package-lock.json
- packages/jsonresume-theme-onepage-plus/package.json
- packages/jsonresume-theme-onepage-plus/resume.json
- packages/jsonresume-theme-onepage-plus/resume.pdf
- pnpm-lock.yaml
Files selected for processing (21)
- .gitignore (1 hunks)
- apps/registry/pages/api/formatters/template.js (2 hunks)
- packages/jsonresume-theme-onepage-plus/.gitignore (1 hunks)
- packages/jsonresume-theme-onepage-plus/.npmignore (1 hunks)
- packages/jsonresume-theme-onepage-plus/README.md (1 hunks)
- packages/jsonresume-theme-onepage-plus/index.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/awards.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/basics.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/certificates.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/education.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/index.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/interests.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/languages.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/projects.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/publications.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/references.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/skills.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/volunteer.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/work.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/resume.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/style.js (1 hunks)
Files skipped from review due to trivial changes (10)
- .gitignore
- packages/jsonresume-theme-onepage-plus/.gitignore
- packages/jsonresume-theme-onepage-plus/.npmignore
- packages/jsonresume-theme-onepage-plus/README.md
- packages/jsonresume-theme-onepage-plus/partials/education.js
- packages/jsonresume-theme-onepage-plus/partials/index.js
- packages/jsonresume-theme-onepage-plus/partials/interests.js
- packages/jsonresume-theme-onepage-plus/partials/languages.js
- packages/jsonresume-theme-onepage-plus/resume.js
- packages/jsonresume-theme-onepage-plus/style.js
Additional comments: 16
apps/registry/pages/api/formatters/template.js (5)
4-4: The import statement for
onepageplushas been added correctly.31-31: The
THEMESobject has been updated to include theonepage-plustheme correctly.28-34: The
getThemefunction correctly handles the retrieval of themes, including the newly addedonepage-plustheme.28-34: The
formatfunction correctly uses thegetThemefunction to render the resume HTML with the specified theme, defaulting to 'elegant' if none is provided.28-34: The
exportsobject is correctly set up to export theformatfunction.packages/jsonresume-theme-onepage-plus/index.js (2)
34-42: The
renderfunction has been updated to includecssandresumeproperties when compiling the template. This change aligns with the PR objectives to reintroduce theonepage-plustheme with updated functionality.45-47: The module exports the
renderfunction, which is consistent with the PR objectives to update the exported public entity for theonepage-plustheme.packages/jsonresume-theme-onepage-plus/partials/basics.js (1)
- 1-51: The template uses Handlebars syntax correctly and is well-structured for readability and maintainability. Ensure that the
idattributes are unique if this template is included multiple times in a single document.packages/jsonresume-theme-onepage-plus/partials/projects.js (5)
1-40: The template uses Handlebars syntax to iterate over
resume.projectsand conditionally render project details. Ensure that theresume.projectsdata is properly sanitized before being rendered to prevent XSS attacks, as the template directly injects data into the HTML.22-22: Verify that the
urlproperty of each project is sanitized to prevent potential security vulnerabilities such as XSS attacks. If theurlis user-supplied data, it should be encoded or validated to ensure it is a safe URL.28-34: The template checks if
highlights.lengthis truthy before iterating overhighlights. This is good practice to prevent rendering empty lists. However, ensure that thehighlightsarray is always an array to avoid runtime errors when calling.lengthon undefined or null.16-20: Ensure that
startDateandendDateare properly sanitized to prevent XSS attacks. If these dates are user-supplied, they should be encoded or validated to ensure they do not contain malicious scripts.35-35: The use of
{{#unless @last}}to conditionally render a separator is a good practice to avoid adding unnecessary markup after the last item in a list.packages/jsonresume-theme-onepage-plus/partials/skills.js (1)
- 1-26: The template for the skills section appears to be correctly structured with proper use of Handlebars syntax for conditionals and loops. Ensure that the data passed to this template is properly sanitized to prevent XSS vulnerabilities, although Handlebars does provide default escaping. The use of conditionals to check for the existence of properties before rendering is a good practice and should help prevent rendering errors.
packages/jsonresume-theme-onepage-plus/partials/work.js (2)
25-25: Verify that
highlightsis always an array before iterating over it to prevent runtime errors ifhighlightsis not iterable.19-19: Ensure that the
urlis properly sanitized before being inserted into thehrefattribute to prevent XSS attacks.
| {{#if reference}} | ||
| <blockquote class="reference"> | ||
| {{reference}} | ||
|
|
||
| {{/if}} | ||
| {{#if name}} | ||
| <div class="name"> | ||
| — {{name}} | ||
| </div> | ||
| {{/if}} | ||
| </blockquote> |
There was a problem hiding this comment.
The blockquote tag should be closed properly regardless of the presence of the name field to ensure valid HTML structure.
{{#if reference}}
<blockquote class="reference">
{{reference}}
{{/if}}
+14:
</blockquote>
{{#if name}}
<div class="name">
— {{name}}
</div>
{{/if}}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.
| {{#if reference}} | |
| <blockquote class="reference"> | |
| {{reference}} | |
| {{/if}} | |
| {{#if name}} | |
| <div class="name"> | |
| — {{name}} | |
| </div> | |
| {{/if}} | |
| </blockquote> | |
| {{#if reference}} | |
| <blockquote class="reference"> | |
| {{reference}} | |
| </blockquote> | |
| {{/if}} | |
| {{#if name}} | |
| <div class="name"> | |
| — {{name}} | |
| </div> | |
| {{/if}} |
| <span class='date'> | ||
| <span class='date'>{{date}}</span> |
There was a problem hiding this comment.
There seems to be redundant nesting of <span> tags with the class 'date'. This could lead to incorrect styling or unexpected layout issues.
- <span class='date'>
- <span class='date'>{{date}}</span>
- </span>
+ <span class='date'>{{date}}</span>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.
| <span class='date'> | |
| <span class='date'>{{date}}</span> | |
| <span class='date'>{{date}}</span> |
| export default `{{#if resume.publications.length}} | ||
| <div class='sectionLine'></div> | ||
| <div id='publications'> | ||
| <div class='sectionName'> | ||
| <span>PUBLICATIONS</span> | ||
| </div> | ||
| <div class='sectionContent'> | ||
| {{#each resume.publications}} | ||
| <div class='blockHeader'> | ||
| <span class='title'> | ||
| {{#if name}}{{name}}{{/if}} | ||
| </span> | ||
| {{#if releaseDate}} | ||
| <span class='date'> | ||
| <span class='releaseDate'>{{formatDate releaseDate}}</span> | ||
| </span> | ||
| {{/if}} | ||
| </div> | ||
| {{#if publisher}} | ||
| <div class="publisher">{{publisher}}</div> | ||
| {{/if}} | ||
| {{#if url}} | ||
| <div class='url'> | ||
| <a href='{{url}}'>{{url}}</a> | ||
| </div> | ||
| {{/if}} | ||
| {{#if summary}} | ||
| <div class="summary"> | ||
| <p>{{summary}}</p> | ||
| </div> | ||
| {{/if}} | ||
| {{#unless @last}}<div class='separator'></div>{{/unless}} | ||
| {{/each}} | ||
| </div> | ||
| </div> | ||
| {{/if}}`; |
There was a problem hiding this comment.
The template for the publications section is well-structured and makes good use of conditional rendering to ensure only available data is displayed. The use of the formatDate helper function on line 15 is consistent with the PR's objectives and helps maintain a clean template. Consider adding a message or fallback content when resume.publications is empty or not present to improve user experience.
| </span> | ||
| {{/location}} | ||
| </div> | ||
| {{#if profiles.length}} |
There was a problem hiding this comment.
Consider verifying that profiles is an array before checking its length to avoid potential runtime errors if profiles is not defined or not an array.
- {{#if profiles.length}}
+ {{#if (isArray profiles) && profiles.length}}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.
| {{#if profiles.length}} | |
| {{#if (isArray profiles) && profiles.length}} |
| Handlebars.registerHelper('formatDate', function (dateString) { | ||
| let dateStrArr = dateString.split('-'); | ||
|
|
||
| if (dateStrArr[0] && dateStrArr[1] && dateStrArr[2]) | ||
| return new Date(dateString).toLocaleDateString('en', { | ||
| month: 'short', | ||
| year: 'numeric', | ||
| day: 'numeric', | ||
| }); | ||
|
|
||
| if (dateStrArr[0] && dateStrArr[1]) | ||
| return new Date(dateString).toLocaleDateString('en', { | ||
| month: 'short', | ||
| year: 'numeric', | ||
| }); | ||
|
|
||
| return dateStrArr; | ||
| }); |
There was a problem hiding this comment.
The formatDate helper function has been updated to handle different date formats. Ensure that this function correctly handles all expected date string formats and consider adding error handling for invalid date strings to avoid potential runtime errors.
Handlebars.registerHelper('formatDate', function (dateString) {
+ if (!/^\d{4}-\d{2}-\d{2}$/.test(dateString) && !/^\d{4}-\d{2}$/.test(dateString)) {
+ throw new Error('Invalid date format');
+ }
let dateStrArr = dateString.split('-');
// ... rest of the function
});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.
| Handlebars.registerHelper('formatDate', function (dateString) { | |
| let dateStrArr = dateString.split('-'); | |
| if (dateStrArr[0] && dateStrArr[1] && dateStrArr[2]) | |
| return new Date(dateString).toLocaleDateString('en', { | |
| month: 'short', | |
| year: 'numeric', | |
| day: 'numeric', | |
| }); | |
| if (dateStrArr[0] && dateStrArr[1]) | |
| return new Date(dateString).toLocaleDateString('en', { | |
| month: 'short', | |
| year: 'numeric', | |
| }); | |
| return dateStrArr; | |
| }); | |
| Handlebars.registerHelper('formatDate', function (dateString) { | |
| if (!/^\d{4}-\d{2}-\d{2}$/.test(dateString) && !/^\d{4}-\d{2}$/.test(dateString)) { | |
| throw new Error('Invalid date format'); | |
| } | |
| let dateStrArr = dateString.split('-'); | |
| if (dateStrArr[0] && dateStrArr[1] && dateStrArr[2]) | |
| return new Date(dateString).toLocaleDateString('en', { | |
| month: 'short', | |
| year: 'numeric', | |
| day: 'numeric', | |
| }); | |
| if (dateStrArr[0] && dateStrArr[1]) | |
| return new Date(dateString).toLocaleDateString('en', { | |
| month: 'short', | |
| year: 'numeric', | |
| }); | |
| return dateStrArr; | |
| }); |
| {{#if position}} | <span class="position">{{position}}</span>{{/if}} | ||
| {{#if startDate}} | ||
| <span class='date'> | ||
| {{#if startDate}}{{startDate}}{{/if}} — {{#if endDate}}{{endDate}}{{else}}Present{{/if}} |
There was a problem hiding this comment.
Consider simplifying the Handlebars expression for startDate since the {{#if startDate}} condition is already being checked, the inner {{#if startDate}} is redundant.
- {{#if startDate}}{{startDate}}{{/if}} — {{#if endDate}}{{endDate}}{{else}}Present{{/if}}
+ {{startDate}} — {{#if endDate}}{{endDate}}{{else}}Present{{/if}}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.
| {{#if startDate}}{{startDate}}{{/if}} — {{#if endDate}}{{endDate}}{{else}}Present{{/if}} | |
| {{startDate}} — {{#if endDate}}{{endDate}}{{else}}Present{{/if}} |
| <span class='title'>{{#if position}}{{position}}{{/if}}</span>{{#if organization}} | {{organization}}{{/if}} | ||
| {{#if startDate}} | ||
| <span class='date'> | ||
| {{#if startDate}}{{startDate}}{{/if}} — {{#if endDate}}{{endDate}}{{else}}Present{{/if}} |
There was a problem hiding this comment.
Consider using the new formatDate helper function to format startDate and endDate for consistency with other parts of the application that may use this helper.
| {{#if url}} | ||
| <div class='url'><a href='{{url}}'>{{url}}</a></div> |
There was a problem hiding this comment.
Ensure that URLs are sanitized before being rendered to prevent potential XSS attacks. If the url is not sanitized elsewhere in the code, it should be handled here.
| {{#if highlights.length}} | ||
| <ul class='highlights'> | ||
| {{#each highlights}} | ||
| <li>{{.}}</li> | ||
| {{/each}} | ||
| </ul> |
There was a problem hiding this comment.
Similar to URLs, ensure that the highlights content is sanitized to prevent XSS vulnerabilities.
alexanderdavide
force-pushed
the
feature/migrate-theme-onepage-plus
branch
from
d0b7c75
to
76f8a00
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (6)
- apps/registry/package.json
- packages/jsonresume-theme-onepage-plus/package-lock.json
- packages/jsonresume-theme-onepage-plus/package.json
- packages/jsonresume-theme-onepage-plus/resume.json
- packages/jsonresume-theme-onepage-plus/resume.pdf
- pnpm-lock.yaml
Files selected for processing (21)
- .gitignore (1 hunks)
- apps/registry/pages/api/formatters/template.js (2 hunks)
- packages/jsonresume-theme-onepage-plus/.gitignore (1 hunks)
- packages/jsonresume-theme-onepage-plus/.npmignore (1 hunks)
- packages/jsonresume-theme-onepage-plus/README.md (1 hunks)
- packages/jsonresume-theme-onepage-plus/index.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/awards.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/basics.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/certificates.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/education.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/index.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/interests.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/languages.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/projects.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/publications.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/references.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/skills.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/volunteer.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/partials/work.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/resume.js (1 hunks)
- packages/jsonresume-theme-onepage-plus/style.js (1 hunks)
Files skipped from review due to trivial changes (15)
- .gitignore
- packages/jsonresume-theme-onepage-plus/.gitignore
- packages/jsonresume-theme-onepage-plus/.npmignore
- packages/jsonresume-theme-onepage-plus/README.md
- packages/jsonresume-theme-onepage-plus/partials/awards.js
- packages/jsonresume-theme-onepage-plus/partials/basics.js
- packages/jsonresume-theme-onepage-plus/partials/education.js
- packages/jsonresume-theme-onepage-plus/partials/index.js
- packages/jsonresume-theme-onepage-plus/partials/interests.js
- packages/jsonresume-theme-onepage-plus/partials/languages.js
- packages/jsonresume-theme-onepage-plus/partials/skills.js
- packages/jsonresume-theme-onepage-plus/partials/volunteer.js
- packages/jsonresume-theme-onepage-plus/partials/work.js
- packages/jsonresume-theme-onepage-plus/resume.js
- packages/jsonresume-theme-onepage-plus/style.js
Files skipped from review as they are similar to previous changes (5)
- apps/registry/pages/api/formatters/template.js
- packages/jsonresume-theme-onepage-plus/index.js
- packages/jsonresume-theme-onepage-plus/partials/projects.js
- packages/jsonresume-theme-onepage-plus/partials/publications.js
- packages/jsonresume-theme-onepage-plus/partials/references.js
| <span class='date'> | ||
| <span class='date'>{{formatDate date}}</span> | ||
| </span> |
There was a problem hiding this comment.
The nested span with the class 'date' is still present, which could lead to styling conflicts or semantic HTML issues. Consider removing the outer span to keep the markup clean.
- <span class='date'>
<span class='date'>{{formatDate date}}</span>
- </span>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.
| <span class='date'> | |
| <span class='date'>{{formatDate date}}</span> | |
| </span> | |
| <span class='date'>{{formatDate date}}</span> |
Hey,
as said in #36, I'd like to have onepage-plus back and thought I'd give it a shot.
I tried to copy your migration of theme 'flat' and also drew inspiration from the implementation of partials in theme 'full'.
Too, I made sure nobody pushes .pnpm-store.
I hope to soon be able to enjoy onepage-plus again.
Summary by CodeRabbit
New Features
formatDatehelper for improved date presentation in resumes.Enhancements
.gitignoreand.npmignorefiles for better project hygiene.Documentation
Style
Bug Fixes
Refactor