-
Notifications
You must be signed in to change notification settings - Fork 0
feat: frontend enhancement #584
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
Conversation
WalkthroughThis pull request updates three Vue components. In the footer component, the copyright text is corrected, and the design/revision credits are split into distinct items with an additional Creative Commons license entry. The About page now uses an external link to a Tumblr URL instead of an internal router link, along with a comment noting future backend API plans. In the Home page, dynamic text functionality and related imports have been commented out, and both the welcome message and link label have been revised. Changes
Possibly related PRs
Suggested labels
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web-frontend/src/pages/Home.vue (1)
2-3: Consider removing commented code instead of keeping itRather than commenting out significant portions of code (imports, configuration, and component usage), it would be better to remove these lines entirely if the VueWriter functionality is no longer needed. This would improve code maintainability and readability.
If you need to reference this code in the future, it's still accessible through version control history.
Also applies to: 6-20, 25-32
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web-frontend/src/components/FooterContent.vue(1 hunks)web-frontend/src/pages/About.vue(1 hunks)web-frontend/src/pages/Home.vue(1 hunks)
🔇 Additional comments (4)
web-frontend/src/components/FooterContent.vue (1)
9-12: Footer attribution improvements look goodThe changes properly separate the different types of attribution (copyright, design, revision) and add the Creative Commons license information, making the footer more informative and correctly structured.
web-frontend/src/pages/Home.vue (3)
37-40: New welcome message looks goodThe updated welcome message provides a more personalized tone that aligns with the other changes in this PR.
44-44: "About me" link label improvementChanging the link text from "Learn more" to "About me" provides better context for users and aligns with the PR's goal of enhancing the frontend.
52-54: Check CSS class applicationNote that the ".top-message" CSS class is now only applied to the h1 element, whereas previously it was also applied to the VueWriter component. Please verify that this is intentional and doesn't affect the styling.
| <div class="button primary"><a href="https://hwakabh.tumblr.com" target="_blank" rel="noopener noreferrer">Blog</a></div> | ||
| <!-- TODO: implement http-clients to tumbler via backend API --> | ||
| <!-- <li><router-link to="/help" class="button primary">Blog</router-link></li> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
HTML structure needs improvement for semantic correctness
The current implementation has two issues:
- Using a
<div>directly inside a<ul>is not semantically correct HTML. List items should always be wrapped in<li>tags. - The "button primary" class is now on the div instead of directly on the interactive element.
- <div class="button primary"><a href="https://hwakabh.tumblr.com" target="_blank" rel="noopener noreferrer">Blog</a></div>
- <!-- TODO: implement http-clients to tumbler via backend API -->
- <!-- <li><router-link to="/help" class="button primary">Blog</router-link></li> -->
+ <li><a href="https://hwakabh.tumblr.com" target="_blank" rel="noopener noreferrer" class="button primary">Blog</a></li>
+ <!-- TODO: implement http-clients to tumbler via backend API -->📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="button primary"><a href="https://hwakabh.tumblr.com" target="_blank" rel="noopener noreferrer">Blog</a></div> | |
| <!-- TODO: implement http-clients to tumbler via backend API --> | |
| <!-- <li><router-link to="/help" class="button primary">Blog</router-link></li> --> | |
| <li><a href="https://hwakabh.tumblr.com" target="_blank" rel="noopener noreferrer" class="button primary">Blog</a></li> | |
| <!-- TODO: implement http-clients to tumbler via backend API --> |
|
Fixed in local 25-04-03 13:25:00 hwakabh.github.io/web-frontend [feat/footers] % npm run test
> web-frontend@0.0.0 test
> vitest run
RUN v3.1.1 /Users/hiro.wakabayashi/git/hwakabh.github.io/web-frontend
✓ tests/FooterContent.test.js (2 tests) 7ms
✓ component: FooterComponent.vue > Component name should be correct 0ms
✓ component: FooterComponent.vue > Component can accept value as props and render as HTML 6ms
Test Files 1 passed (1)
Tests 2 passed (2)
Start at 13:25:06
Duration 374ms (transform 37ms, setup 0ms, collect 54ms, tests 7ms, environment 134ms, prepare 51ms) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web-frontend/tests/FooterContent.test.js (1)
10-10: Consider adding proper spacing in the expected string.The expected string appears to be missing spaces between different sections:
- Between "reserved." and "Design"
- Between "UP" and "Revised"
- Between "hwakabh" and "released"
This could make the test brittle if the actual component renders with proper spacing. Consider modifying it to match the actual spacing in the rendered component:
- const expected = "© hwakabh. All rights reserved.Design: HTML5 UPRevised: hwakabhreleased for free under the Creative Commons"; + const expected = "© hwakabh. All rights reserved. Design: HTML5 UP Revised: hwakabh released for free under the Creative Commons";Additionally, since the AI summary indicates that the footer content was restructured to include separate list items, you might want to consider testing the structure of the component rather than just the text content.
* feat: frontend enhancement (#584) * chore: moved license texts into footer. * chore: updated href for blogs. * test: updated expected values. * build(deps-dev): bump vite in /web-frontend in the non-majors group (#586) Bumps the non-majors group in /web-frontend with 1 update: [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite). Updates `vite` from 6.2.4 to 6.2.5 - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/v6.2.5/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v6.2.5/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 6.2.5 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: non-majors ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Issue/PR link
N/A
What does this PR do?
Describe what changes you make in your branch:
(Optional) Additional Contexts
Describe additional information for reviewers (i.e. What does not included)
mainto confirm changes would be appliedSummary by CodeRabbit