Add export popup with JPG/PDF and per-epic options - #2
Open
FR073N wants to merge 2 commits into
Open
Conversation
Replace the single "Export PNG" button with an "Export" button that opens a popup modal. Users can choose between JPG and PDF formats, with options for global view and/or per-epic export. JPG per-epic exports as a ZIP file. PDF renders all pages in landscape A4 with centered content and background fill. - Add export popup UI (format toggle, checkboxes, export button) - Add jsPDF and JSZip lazy-loaders (CDN, same pattern as html2canvas) - Refactor doExport into reusable renderBoardToCanvas function - Render at 3x scale with 0.98 JPEG quality for sharp output Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughRemplacement de l'action "Export PNG" par une popup d'export (JPG/PDF) avec options "Vue globale" et "Par epic", pipeline de rendu partagé passant à canvas 3x, lazy-loading de jsPDF/JSZip, et génération de JPEG(s) ou PDF(s) multi-pages; ajout de styles et d'un workflow de release + script de publication. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Popup Export
participant Renderer as Pipeline Rendu
participant Canvas as Canvas 3x
participant Exporter as Exporteur (JPG/PDF)
participant Browser as Téléchargement
User->>UI: Ouvre popup / choisit format & options
UI->>Renderer: Demande rendu (sélections, options)
Renderer->>Renderer: Calcule bounds & layout
Renderer->>Canvas: Crée canvas (3x) et rasterise
Canvas-->>Renderer: Retour canvas rendu
Renderer->>Exporter: Fournit canvas(s) & métadonnées
alt JPG
Exporter->>Exporter: Génère JPEG(s), crée ZIP si multiple
Exporter-->>Browser: Fournit ZIP ou .jpg
else PDF
Exporter->>Exporter: Charge jsPDF, compose pages A4 paysage
Exporter-->>Browser: Fournit .pdf
end
Browser-->>User: Démarre téléchargement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flowboard.css`:
- Around line 482-486: Rename the CSS state class .fb-export-popup-format.active
to .fb-export-popup-format.fb-active and update all JS references that
add/remove/toggle the "active" class in the Flowboard code (the block around
flowboard.js handling export popup state, lines ~1840-1859) to use "fb-active"
instead (including classList.add/remove/toggle or jQuery
.addClass/.removeClass/.toggleClass calls) so the selector and runtime class
changes remain consistent with the fb- prefix convention.
In `@flowboard.js`:
- Around line 1988-2015: computeExportBounds currently uses global state
(buildSpreadMap, resolveArrowSides, getAnchor and state.svgEl) so arrow geometry
and SVG fragments are taken from the live board instead of the passed-in local
export data; change it to compute arrow geometry solely from the function
arguments: use the passed-in arrows and positions to build a local spreadMap (do
not call or rely on global buildSpreadMap/state.project.arrows), compute
per-arrow sides with a local resolveArrowSides-like routine that accepts the
local arrows and positions, derive anchors from the supplied positions (not
state.screenEls/getAnchor), and ensure renderBoardToCanvas is given the
rebuilt/export-only SVG (reconstructed from the arrows+positions) rather than
cloning state.svgEl; apply the same change to the similar block around the
2055-2065 region so all export-by-epic code uses only the provided
arrows/positions.
- Around line 38-39: Lors de FlowBoard.init(), réinitialisez toute référence de
popup persistante en appelant closeExportPopup() si nécessaire et en faisant
state.exportPopup = null (et éventuellement state.screenPopup = null) pour
garantir que l'ancienne référence DOM et les écouteurs globaux ne restent pas
actifs; localisez la fonction init() dans FlowBoard et ajoutez ces appels avant
de ré-attacher les écouteurs ou de créer une nouvelle popup afin d'éviter que le
test if (state.exportPopup) { closeExportPopup(); return; } bloque l'ouverture
après un init() intermédiaire.
- Around line 1957-1985: Supprimez les loaders runtime jsPDF/JSZip (jsPDFLoaded,
jsZipLoaded, loadJsPDF, loadJSZip) et remplacez-les par des implémentations sans
dépendances externes : pour l'export PDF, n'utilisez plus loadJsPDF but
implémentez une exportation basée sur les API natives (rendre le canvas/SVG en
image via toBlob()/toDataURL puis créer un Blob et déclencher un téléchargement
via un <a> temporaire ou utiliser la fenêtre d'impression) ; pour
l'archivage/remise de plusieurs fichiers, retirez loadJSZip et fournissez soit
des téléchargements individuels via Blob+anchor, soit une implémentation locale
intégrée (ou un fallback server-side) plutôt que de charger JSZip depuis un CDN
; nettoyez toutes les références à jsPDFLoaded/jsZipLoaded et adaptez les
fonctions d'export existantes pour utiliser ces nouveaux flux natifs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08ba8a7e-c861-42f3-b3c0-ff156b6c5ffb
📒 Files selected for processing (2)
flowboard.cssflowboard.js
Comment on lines
+482
to
+486
| .fb-export-popup-format.active { | ||
| background: #2A9D8F; | ||
| color: #fff; | ||
| border-color: #2A9D8F; | ||
| } |
There was a problem hiding this comment.
Remplacez la classe d’état active par un nom préfixé.
active enfreint la convention fb- et reste exposée aux styles globaux. Ici, renommez-la en fb-active côté CSS et alignez aussi les ajouts/retraits de classe dans flowboard.js:1840-1859.
As per coding guidelines "All CSS classes must use the fb- prefix to avoid collisions with external stylesheets."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flowboard.css` around lines 482 - 486, Rename the CSS state class
.fb-export-popup-format.active to .fb-export-popup-format.fb-active and update
all JS references that add/remove/toggle the "active" class in the Flowboard
code (the block around flowboard.js handling export popup state, lines
~1840-1859) to use "fb-active" instead (including classList.add/remove/toggle or
jQuery .addClass/.removeClass/.toggleClass calls) so the selector and runtime
class changes remain consistent with the fb- prefix convention.
Comment on lines
+38
to
+39
| screenPopup: null, | ||
| exportPopup: null |
There was a problem hiding this comment.
Réinitialisez state.exportPopup pendant init().
Cette nouvelle référence DOM reste vivante entre deux FlowBoard.init(). Si un init() arrive pendant que la popup est ouverte, le clic suivant sur “Export” tombe dans if (state.exportPopup) { closeExportPopup(); return; } et n’ouvre rien, avec l’écouteur global potentiellement encore attaché. Ajoutez au minimum un closeExportPopup()/state.exportPopup = null dans l’initialisation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flowboard.js` around lines 38 - 39, Lors de FlowBoard.init(), réinitialisez
toute référence de popup persistante en appelant closeExportPopup() si
nécessaire et en faisant state.exportPopup = null (et éventuellement
state.screenPopup = null) pour garantir que l'ancienne référence DOM et les
écouteurs globaux ne restent pas actifs; localisez la fonction init() dans
FlowBoard et ajoutez ces appels avant de ré-attacher les écouteurs ou de créer
une nouvelle popup afin d'éviter que le test if (state.exportPopup) {
closeExportPopup(); return; } bloque l'ouverture après un init() intermédiaire.
Comment on lines
+1957
to
+1985
| var jsPDFLoaded = null; | ||
|
|
||
| function loadJsPDF() { | ||
| if (jsPDFLoaded) return jsPDFLoaded; | ||
| jsPDFLoaded = new Promise(function (resolve, reject) { | ||
| if (window.jspdf) { resolve(window.jspdf); return; } | ||
| var s = document.createElement('script'); | ||
| s.src = 'https://cdn.jsdelivr.net/npm/jspdf@2.5.1/dist/jspdf.umd.min.js'; | ||
| s.onload = function () { resolve(window.jspdf); }; | ||
| s.onerror = function () { jsPDFLoaded = null; reject(new Error('Failed to load jsPDF')); }; | ||
| document.head.appendChild(s); | ||
| }); | ||
| return jsPDFLoaded; | ||
| } | ||
|
|
||
| var jsZipLoaded = null; | ||
|
|
||
| function loadJSZip() { | ||
| if (jsZipLoaded) return jsZipLoaded; | ||
| jsZipLoaded = new Promise(function (resolve, reject) { | ||
| if (window.JSZip) { resolve(window.JSZip); return; } | ||
| var s = document.createElement('script'); | ||
| s.src = 'https://cdn.jsdelivr.net/npm/jszip@3.10.1/dist/jszip.min.js'; | ||
| s.onload = function () { resolve(window.JSZip); }; | ||
| s.onerror = function () { jsZipLoaded = null; reject(new Error('Failed to load JSZip')); }; | ||
| document.head.appendChild(s); | ||
| }); | ||
| return jsZipLoaded; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if flowboard.js exists and get its size
fd -t f 'flowboard.js' | head -1Repository: snowpact/html-flow-board
Length of output: 81
🏁 Script executed:
# Read the specific lines mentioned in the review
wc -l flowboard.jsRepository: snowpact/html-flow-board
Length of output: 86
🏁 Script executed:
# Read lines 1957-1985 from flowboard.js
sed -n '1957,1985p' flowboard.js | cat -nRepository: snowpact/html-flow-board
Length of output: 1401
Supprimer les dépendances runtime depuis CDN (jsPDF et JSZip).
Le code aux lignes 1957-1985 viole la contrainte « pas de dépendances externes ». Le chargement de jsPDF et JSZip depuis un CDN introduit des dépendances réseau, de disponibilité du CDN et de politiques CSP de la page hôte. Implémenter ces fonctionnalités en vanilla JavaScript ou réduire le scope de l'export pour respecter la contrainte de zéro dépendance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flowboard.js` around lines 1957 - 1985, Supprimez les loaders runtime
jsPDF/JSZip (jsPDFLoaded, jsZipLoaded, loadJsPDF, loadJSZip) et remplacez-les
par des implémentations sans dépendances externes : pour l'export PDF,
n'utilisez plus loadJsPDF but implémentez une exportation basée sur les API
natives (rendre le canvas/SVG en image via toBlob()/toDataURL puis créer un Blob
et déclencher un téléchargement via un <a> temporaire ou utiliser la fenêtre
d'impression) ; pour l'archivage/remise de plusieurs fichiers, retirez loadJSZip
et fournissez soit des téléchargements individuels via Blob+anchor, soit une
implémentation locale intégrée (ou un fallback server-side) plutôt que de
charger JSZip depuis un CDN ; nettoyez toutes les références à
jsPDFLoaded/jsZipLoaded et adaptez les fonctions d'export existantes pour
utiliser ces nouveaux flux natifs.
Comment on lines
+1988
to
2015
| function computeExportBounds(screens, positions, arrows) { | ||
| var minX = Infinity, minY = Infinity, maxX = 0, maxY = 0; | ||
| var arrows = state.project.arrows || []; | ||
| var spreadMap = buildSpreadMap(); | ||
|
|
||
| state.project.screens.forEach(function (s) { | ||
| if (state.hiddenScreens[s.id]) return; | ||
| screens.forEach(function (s) { | ||
| var el = state.screenEls[s.id]; | ||
| var pos = state.positions[s.id]; | ||
| var pos = positions[s.id]; | ||
| if (!el || !pos) return; | ||
| minX = Math.min(minX, pos.x); | ||
| minY = Math.min(minY, pos.y); | ||
| maxX = Math.max(maxX, pos.x + el.offsetWidth); | ||
| maxY = Math.max(maxY, pos.y + el.offsetHeight); | ||
| }); | ||
|
|
||
| // Include arrow control points so arrows aren't clipped | ||
| arrows.forEach(function (arrow, idx) { | ||
| if (state.hiddenScreens[arrow.from] || state.hiddenScreens[arrow.to]) return; | ||
|
|
||
| var fromEl = state.screenEls[arrow.from]; | ||
| var toEl = state.screenEls[arrow.to]; | ||
| if (!fromEl || !toEl) return; | ||
|
|
||
| var sides = resolveArrowSides(arrow, idx, spreadMap); | ||
| var allArrows = state.project.arrows || []; | ||
| var globalIdx = -1; | ||
| for (var i = 0; i < allArrows.length; i++) { | ||
| if (allArrows[i] === arrow) { globalIdx = i; break; } | ||
| } | ||
| if (globalIdx < 0) globalIdx = idx; | ||
|
|
||
| var sides = resolveArrowSides(arrow, globalIdx, spreadMap); | ||
| var start = getAnchor(arrow.from, sides.from); |
There was a problem hiding this comment.
L’export “Par epic” réutilise encore la géométrie de la vue globale.
getEpicExportData() fournit des positions locales, mais ici la géométrie des flèches continue de dépendre de l’état global (buildSpreadMap()/resolveArrowSides()/getAnchor()) puis renderBoardToCanvas() clone state.svgEl complet. Sur un export par epic, les cartes sont donc repositionnées alors que les flèches restent celles du board courant, avec risque de décalage, de recadrage énorme ou d’inclusion de liaisons hors epic. Il faut reconstruire le SVG d’export à partir de arrows + positions passés en argument, au lieu de réutiliser state.svgEl.
Also applies to: 2055-2065
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flowboard.js` around lines 1988 - 2015, computeExportBounds currently uses
global state (buildSpreadMap, resolveArrowSides, getAnchor and state.svgEl) so
arrow geometry and SVG fragments are taken from the live board instead of the
passed-in local export data; change it to compute arrow geometry solely from the
function arguments: use the passed-in arrows and positions to build a local
spreadMap (do not call or rely on global buildSpreadMap/state.project.arrows),
compute per-arrow sides with a local resolveArrowSides-like routine that accepts
the local arrows and positions, derive anchors from the supplied positions (not
state.screenEls/getAnchor), and ensure renderBoardToCanvas is given the
rebuilt/export-only SVG (reconstructed from the arrows+positions) rather than
cloning state.svgEl; apply the same change to the similar block around the
2055-2065 region so all export-by-epic code uses only the provided
arrows/positions.
- GitHub Action: on tag push v*, runs tests then creates a GitHub Release with auto-generated release notes and attaches flowboard.js/css - npm run release -- <patch|minor|major>: bumps version, commits, tags, pushes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/release.sh`:
- Line 50: Replace the unsafe git push command that uses --tags (the line
containing "git push origin main --tags") so you only publish the branch and the
specific release tag: first push the branch ("git push origin main") then push
the single release tag variable used in the script (e.g. "git push origin
\"$RELEASE_TAG\"" or the script's tag variable name). Ensure the tag variable
(RELEASE_TAG or TAG) is set earlier in the script before you push it.
- Around line 15-36: Validate the BUMP argument (the $1 variable / BUMP) before
computing NEW_VERSION: check that BUMP is one of the allowed values "major",
"minor" or "patch" and if not print a clear error and exit non‑zero, then
proceed to the version calculation; update the release.sh logic to perform this
explicit validation immediately after setting BUMP (before the NEW_VERSION
block) so invalid inputs fail fast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82a2f3ed-c579-4918-b3e2-c07aabfd261f
📒 Files selected for processing (3)
.github/workflows/release.ymlpackage.jsonscripts/release.sh
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/release.yml
- package.json
Comment on lines
+15
to
+36
| BUMP=$1 | ||
|
|
||
| if [ "$(git status --porcelain)" ]; then | ||
| echo "Error: working directory not clean. Commit or stash changes first." | ||
| exit 1 | ||
| fi | ||
|
|
||
| BRANCH=$(git rev-parse --abbrev-ref HEAD) | ||
| if [ "$BRANCH" != "main" ]; then | ||
| echo "Error: must be on main branch (currently on $BRANCH)" | ||
| exit 1 | ||
| fi | ||
|
|
||
| git pull --ff-only origin main | ||
|
|
||
| NEW_VERSION=$(node -p " | ||
| var v = '$CURRENT_VERSION'.split('.').map(Number); | ||
| if ('$BUMP' === 'major') { v[0]++; v[1]=0; v[2]=0; } | ||
| else if ('$BUMP' === 'minor') { v[1]++; v[2]=0; } | ||
| else if ('$BUMP' === 'patch') { v[2]++; } | ||
| v.join('.'); | ||
| ") |
There was a problem hiding this comment.
Valider explicitement le type de bump avant le calcul de version.
Actuellement, une valeur invalide de $1 passe le flux jusqu’au commit/tag, au lieu d’échouer immédiatement avec un message clair.
✅ Correctif proposé
BUMP=$1
+
+case "$BUMP" in
+ patch|minor|major) ;;
+ *)
+ echo "Error: invalid bump type '$BUMP'. Use one of: patch|minor|major"
+ exit 1
+ ;;
+esac
if [ "$(git status --porcelain)" ]; then
echo "Error: working directory not clean. Commit or stash changes first."
exit 1
fi📝 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.
Suggested change
| BUMP=$1 | |
| if [ "$(git status --porcelain)" ]; then | |
| echo "Error: working directory not clean. Commit or stash changes first." | |
| exit 1 | |
| fi | |
| BRANCH=$(git rev-parse --abbrev-ref HEAD) | |
| if [ "$BRANCH" != "main" ]; then | |
| echo "Error: must be on main branch (currently on $BRANCH)" | |
| exit 1 | |
| fi | |
| git pull --ff-only origin main | |
| NEW_VERSION=$(node -p " | |
| var v = '$CURRENT_VERSION'.split('.').map(Number); | |
| if ('$BUMP' === 'major') { v[0]++; v[1]=0; v[2]=0; } | |
| else if ('$BUMP' === 'minor') { v[1]++; v[2]=0; } | |
| else if ('$BUMP' === 'patch') { v[2]++; } | |
| v.join('.'); | |
| ") | |
| BUMP=$1 | |
| case "$BUMP" in | |
| patch|minor|major) ;; | |
| *) | |
| echo "Error: invalid bump type '$BUMP'. Use one of: patch|minor|major" | |
| exit 1 | |
| ;; | |
| esac | |
| if [ "$(git status --porcelain)" ]; then | |
| echo "Error: working directory not clean. Commit or stash changes first." | |
| exit 1 | |
| fi | |
| BRANCH=$(git rev-parse --abbrev-ref HEAD) | |
| if [ "$BRANCH" != "main" ]; then | |
| echo "Error: must be on main branch (currently on $BRANCH)" | |
| exit 1 | |
| fi | |
| git pull --ff-only origin main | |
| NEW_VERSION=$(node -p " | |
| var v = '$CURRENT_VERSION'.split('.').map(Number); | |
| if ('$BUMP' === 'major') { v[0]++; v[1]=0; v[2]=0; } | |
| else if ('$BUMP' === 'minor') { v[1]++; v[2]=0; } | |
| else if ('$BUMP' === 'patch') { v[2]++; } | |
| v.join('.'); | |
| ") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/release.sh` around lines 15 - 36, Validate the BUMP argument (the $1
variable / BUMP) before computing NEW_VERSION: check that BUMP is one of the
allowed values "major", "minor" or "patch" and if not print a clear error and
exit non‑zero, then proceed to the version calculation; update the release.sh
logic to perform this explicit validation immediately after setting BUMP (before
the NEW_VERSION block) so invalid inputs fail fast.
| git add package.json | ||
| git commit -m "Release v$NEW_VERSION" | ||
| git tag "v$NEW_VERSION" | ||
| git push origin main --tags |
There was a problem hiding this comment.
Éviter --tags: cela peut publier des tags non liés à cette release.
Avec le workflow déclenché sur v*, pousser tous les tags locaux peut lancer des releases GitHub inattendues.
✅ Correctif proposé
-git push origin main --tags
+git push origin main "v$NEW_VERSION"📝 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.
Suggested change
| git push origin main --tags | |
| git push origin main "v$NEW_VERSION" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/release.sh` at line 50, Replace the unsafe git push command that uses
--tags (the line containing "git push origin main --tags") so you only publish
the branch and the specific release tag: first push the branch ("git push origin
main") then push the single release tag variable used in the script (e.g. "git
push origin \"$RELEASE_TAG\"" or the script's tag variable name). Ensure the tag
variable (RELEASE_TAG or TAG) is set earlier in the script before you push it.
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
1 participant
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
renderBoardToCanvas()functionTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Notes de version
Nouvelles fonctionnalités
Chores