fix: resolve visual, accessibility, logic, and deployment issues from full audit#112
Merged
Merged
Conversation
… audit Visual fixes: - Fix active tab 1px vertical jump by using transparent bottom border - Smooth mobile menu overlay transitions using opacity/visibility instead of display Accessibility: - Add aria-hidden="true" to 11 modal dialogs missing the attribute Logic fixes: - Fix Find & Replace capture group corruption by wrapping replacement in arrow function - Align HTML export Mermaid version from v10.9.1 to v11.6.0 Deployment: - Exclude desktop-app/ from Docker build context via .dockerignore - Guard CI/CD registry push to prevent publishing on pull_request events
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ThisIs-Developer
added
bug
There was a problem hiding this comment.
Pull request overview
This PR applies a set of small fixes found during an audit, spanning UI polish, accessibility attributes on dialogs, a find/replace correctness fix, Mermaid export consistency, and CI/Docker hardening.
Changes:
- CSS: prevent active-tab “1px jump” and smooth the mobile menu overlay fade.
- HTML/JS: add
aria-hiddendefaults to dialogs; fix literal replacement behavior in “Replace all”; align Mermaid CDN version in exported HTML. - Ops: exclude
desktop-app/from Docker build context and avoid pushing images onpull_requestevents.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| styles.css | Reserves tab border space to avoid layout jump; replaces overlay display-toggling with opacity/visibility transitions. |
| index.html | Adds aria-hidden="true" to several dialog wrappers so hidden dialogs aren’t announced by screen readers. |
| script.js | Makes replace-all treat user replacement text literally; updates Mermaid version in the HTML export template. |
| .github/workflows/docker-publish.yml | Prevents GHCR pushes on PR events to reduce CI failures on forks. |
| .dockerignore | Excludes desktop-app/ from the web Docker image context to reduce bloat and leakage of dev artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
959
to
970
| @@ -966,14 +965,15 @@ a:focus { | |||
| background-color: rgba(0, 0, 0, 0.5); | |||
| opacity: 0; | |||
| visibility: hidden; | |||
| pointer-events: none; | |||
| transition: opacity 0.3s ease, visibility 0.3s ease; | |||
| z-index: 1000; | |||
Comment on lines
1667
to
1671
| max-width: 180px; | ||
| background-color: var(--button-bg); | ||
| border: 1px solid var(--border-color); | ||
| border-bottom: none; | ||
| border-bottom: 1px solid transparent; | ||
| border-radius: 6px 6px 0 0; |
Comment on lines
3769
to
3774
| if (!query) return; | ||
| const replacement = findReplaceWith ? findReplaceWith.value : ''; | ||
| const regex = new RegExp(escapeRegExp(query), 'gi'); | ||
| markdownEditor.value = markdownEditor.value.replace(regex, replacement); | ||
| markdownEditor.value = markdownEditor.value.replace(regex, () => replacement); | ||
| markdownEditor.dispatchEvent(new Event('input', { bubbles: true })); | ||
| refreshFindMatches({ resetIndex: true }); |
Comment on lines
4330
to
4334
| }; | ||
| </script> | ||
| <script defer src="https://cdnjs.cloudflare.com/ajax/libs/mathjax/3.2.2/es5/tex-mml-chtml.min.js"></script> | ||
| <script src="https://cdn.jsdelivr.net/npm/mermaid@10.9.1/dist/mermaid.min.js"></script> | ||
| <script src="https://cdn.jsdelivr.net/npm/mermaid@11.6.0/dist/mermaid.min.js"></script> | ||
| <style> |
Comment on lines
541
to
545
| <!-- Link Modal --> | ||
| <div id="link-modal" class="reset-modal-overlay" role="dialog" aria-modal="true" aria-labelledby="link-modal-title" style="display:none;"> | ||
| <div id="link-modal" class="reset-modal-overlay" role="dialog" aria-modal="true" aria-labelledby="link-modal-title" aria-hidden="true" style="display:none;"> | ||
| <div class="reset-modal-box reset-modal-box--wide"> | ||
| <p id="link-modal-title" class="reset-modal-message">Insert link</p> | ||
| <div class="reset-modal-field"> |
|
|
||
| <!-- Mermaid Zoom Modal --> | ||
| <div id="mermaid-zoom-modal" role="dialog" aria-modal="true" aria-label="Diagram zoom view"> | ||
| <div id="mermaid-zoom-modal" role="dialog" aria-modal="true" aria-label="Diagram zoom view" aria-hidden="true"> |
Comment on lines
314
to
317
| <!-- Reset Confirmation Modal --> | ||
| <div id="reset-confirm-modal" class="reset-modal-overlay" role="dialog" aria-modal="true" aria-labelledby="reset-modal-title" style="display:none;"> | ||
| <div id="reset-confirm-modal" class="reset-modal-overlay" role="dialog" aria-modal="true" aria-labelledby="reset-modal-title" aria-hidden="true" style="display:none;"> | ||
| <div class="reset-modal-box reset-modal-box--wide"> | ||
| <p id="reset-modal-title" class="reset-modal-message">Are you sure you want to delete all files?</p> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses 6 bugs across 5 files discovered during a comprehensive audit of the
mainbranch. Changes span frontend visuals, accessibility compliance, application logic, and deployment configuration.5 files changed · 20 insertions · 17 deletions
Changes
🎨 Visual Fixes —
styles.cssFix 1: Active Tab Vertical Jump
.tab-itemusedborder-bottom: nonewhile.tab-item.activeapplied a 1px solid bottom border, creating inconsistent box sizing..tab-itemtoborder-bottom: 1px solid transparentso the space is always reserved, eliminating the jump.Fix 2: Mobile Menu Overlay Transition
display: none/display: blocktoggling, which prevented CSStransitionanimations from firing (instant flash instead of smooth fade).displaytoggling withopacity+visibility+pointer-eventsfor a smooth 300ms fade transition..mobile-menu-overlay { - display: none; opacity: 0; visibility: hidden; + pointer-events: none; transition: opacity 0.3s ease, visibility 0.3s ease; } .mobile-menu-overlay.active { - display: block; opacity: 1; visibility: visible; + pointer-events: auto; }♿ Accessibility —
index.htmlFix 3: Missing Modal
aria-hiddenAttributesaria-hidden="true"on their initially-hidden wrapper elements, causing screen readers to announce off-screen content to users.aria-hidden="true"to all 11 affected modals:reset-confirm-modalrename-modallink-modalreference-modalimage-modaltable-modalemoji-modalsymbols-modalalert-modalgithub-import-modalmermaid-zoom-modal🔧 Logic Fixes —
script.jsFix 4: Find & Replace Capture Group Corruption
replaceAllMatches()function passed the replacement string directly toString.prototype.replace(), causing regex special sequences ($1,$&,$$, etc.) typed by users as literal text to be interpreted as capture group references — silently corrupting editor content.() => replacementto ensure literal string substitution.Fix 5: HTML Export Mermaid Version Drift
v10.9.1via CDN while the main application loadsv11.6.0, causing rendering inconsistencies (different syntax support, theme behavior) in exported files.mermaid@11.6.0.� Deployment Hardening
Fix 6a: Docker Build Context —
.dockerignoredesktop-app/directory (NeutralinoJS native binaries, logs, developer configs) was being copied into the Nginx web container via the overly broadCOPY .instruction, bloating the image size and exposing developer artifacts in the web root.desktop-app/exclusion to.dockerignore.Fix 6b: CI/CD Push Guard —
.github/workflows/docker-publish.ymlpush: trueunconditionally attempted to push container images to GHCR on all trigger events, includingpull_requestfrom forks — which would fail due to missing write permissions and produce confusing CI failures.${{ github.event_name != 'pull_request' }}.Testing
role="dialog"havearia-hidden() => replacementmermaid@11.6.0mermaid@10.9.1desktop-app/in.dockerignoredesktop-app/Impact