Skip to content

Implemented pop-up functionality for help modal.#124

Merged
UcheWendy merged 5 commits intomainfrom
121-implement-help-modal-and-basic-ui-polish
Sep 19, 2025
Merged

Implemented pop-up functionality for help modal.#124
UcheWendy merged 5 commits intomainfrom
121-implement-help-modal-and-basic-ui-polish

Conversation

@Joishee05
Copy link
Collaborator

Description

Fixes #121

1. What was Changed

A user-friendly help modal was added to the application. When users click the "Help" button on the sidebar, a pop-up modal appears with instructions on how to use the bone set viewer. The modal includes a title, a brief description, and a close button.

2. Why it was Changed

The goal was to get a basic help button design down before making any complex changes. This change was needed to improve the application's usability and accessibility. Previously, the "Help" link was non-functional, which is not beneficial for end-users. By providing clear instructions in a modal, users can quickly understand how to interact with the interface.

3. How it was Changed

There was no operating help modal before this pull request. The basic modal is hidden by default and is displayed using a flex overlay when the "Help" link is clicked. The close button hides the modal.

@Joishee05 Joishee05 linked an issue Sep 15, 2025 that may be closed by this pull request
@UcheWendy
Copy link
Collaborator

can you resolve the issue with the failed lint check so it passes our workflow setup

const closeHelpModal = document.getElementById('close-help-modal');
if (helpButton && helpModal && closeHelpModal) {
helpButton.addEventListener('click', () => {
helpModal.style.display = 'flex';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would recommend toggling a class or the hidden attribute rather than manipulating style.display—it’s easier to animate and keeps the presentation in CSS.

Copy link
Collaborator

@Taktar Taktar left a comment

Choose a reason for hiding this comment

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

Fix remaining lint failures

CI shows “Strings must use doublequote” for some JS files. Even if they weren’t changed here, our checks run across the workspace. Running npm run lint -- --fix locally and committing the quote fixes would unblock the workflow.

@UcheWendy
Copy link
Collaborator

review the changes requested, add unit test for your code in test suite and ensure workflow error is corrected

Copy link
Collaborator

@UcheWendy UcheWendy left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this PR. The modal is functionally working, which is a great start. I've gone through the files and have some feedback focused on improving the code's structure, maintainability, and robustness which i have added as inline comments.

Also, To ensure this feature remains stable as we add more code, we need to add automated tests.

Request: Please add a new test suite to templates/test.js for the "Help Modal". The tests should verify that:
The modal is hidden by default when the page loads.
The modal becomes visible after the "Help" link is clicked.
The modal becomes hidden again after the "Close" button is clicked.

This will help us catch any regressions in the future.

Copy link
Collaborator

@UcheWendy UcheWendy left a comment

Choose a reason for hiding this comment

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

good work. creating a html fie for the help model leaves room for updating infomation in the future without making changes that will affect other files. i also like the changes and improvement on the css stlying.

@UcheWendy UcheWendy merged commit 43b69ce into main Sep 19, 2025
5 checks passed
@UcheWendy UcheWendy deleted the 121-implement-help-modal-and-basic-ui-polish branch December 8, 2025 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement "Help" Modal and Basic UI Polish

3 participants