Skip to content

Fix mobile signature placement flow, save visibility, and coordinate serialization#7342

Open
Phillipxh wants to merge 4 commits intoLibreSign:mainfrom
Phillipxh:fix/mobile-signature-placement
Open

Fix mobile signature placement flow, save visibility, and coordinate serialization#7342
Phillipxh wants to merge 4 commits intoLibreSign:mainfrom
Phillipxh:fix/mobile-signature-placement

Conversation

@Phillipxh
Copy link
Copy Markdown

📝 Summary

This PR resolves several mobile UX issues and fixes a coordinate serialization bug affecting signature placement:

  • Ensures Save/Sign actions remain accessible on mobile via a sticky action area.
  • Fixes signature placement on mobile, completing on the first tap (no additional tap/drag required).
  • Corrects signature coordinate serialization, aligning with backend/API expectations.
  • Eliminates Chrome passive event listener warnings during touch drag by applying a build-time patch to @libresign/pdf-elements.

🧪 How to Test

  1. Checkout this branch and run:

    npm install
    npm run dev
  2. Open LibreSign and navigate to the signature position modal for a document.

On Mobile Viewport

  • Select a signer.

  • Tap once on the PDF to place the signature.

    • ✅ Signature is placed immediately (no second tap required).
    • ✅ Save button is immediately available.

Sticky Actions

  • Add multiple signers to create a long sidebar.

    • ✅ Save/Sign buttons remain visible and accessible at the bottom.

Drag & Interaction

  • Move an existing signature.

    • ✅ Dragging works smoothly.
    • ✅ No passive-listener warnings appear in the console.

Persistence

  • Save and reload the document.

    • ✅ Signature coordinates persist correctly (not reset to zero).

🎨 UI / Frontend Changes

  • Implemented mobile sticky action bar in VisibleElements.vue
  • Fixed first-tap placement logic and improved signer flow in PdfEditor.vue
  • Mitigated touchmove passive warnings via patch in vite.config.mjs
  • (Optional) Screenshots before/after included
  • (Optional) Tested across multiple browsers
  • (Optional) Accessibility reviewed

⚙️ API / Backend Changes

  • Fixed coordinate serialization in src/store/files.js
  • Ensured compatibility with backend/API expectations
  • (Optional) Added unit/integration tests

✅ Checklist

  • Contribution guide followed
  • npm run dev builds successfully
  • Mobile placement manually verified
  • No desktop regressions observed

🤖 AI Usage

Parts of this PR were generated or assisted by AI.

@github-project-automation github-project-automation bot moved this to 0. Needs triage in Roadmap Mar 29, 2026
@welcome
Copy link
Copy Markdown

welcome bot commented Mar 29, 2026

Thanks for opening your first pull request in this repository! ✌️

@Phillipxh
Copy link
Copy Markdown
Author

@maintainers could you please approve workflows for this first-time contributor PR?

@Phillipxh Phillipxh force-pushed the fix/mobile-signature-placement branch from 4667bbb to f6ccad9 Compare March 29, 2026 14:28
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 24.44444% with 34 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/components/PdfEditor/PdfEditor.vue 17.94% 29 Missing and 3 partials ⚠️
src/components/Request/VisibleElements.vue 66.66% 2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Files with missing lines Coverage Δ
src/store/files.js 74.38% <ø> (ø)
src/components/Request/VisibleElements.vue 56.41% <66.66%> (ø)
src/components/PdfEditor/PdfEditor.vue 56.66% <17.94%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Phillipxh Phillipxh force-pushed the fix/mobile-signature-placement branch from 985f29e to 2a18478 Compare March 30, 2026 20:09
Signed-off-by: Phillip <phillipxh@gmail.com>
Copy link
Copy Markdown
Author

Update: I pushed additional test coverage for the mobile signature placement changes (PdfEditor + VisibleElements specs).

Local verification:

  • vitest run src/tests/components/PdfEditor/PdfEditor.spec.ts src/tests/components/Request/VisibleElements.spec.ts
  • Result: 2 passed, 124 passed

Could a maintainer please approve and run the pending workflows for this fork PR so CI/Codecov can refresh on the latest commit? Thanks!

Copy link
Copy Markdown
Author

Quick status update on the conventional-commits check:

  • The failing run is an older run on commit 985f29ea... that still contained Fix mobile signature placement and coordinate serialization (non-conventional format).
  • The current branch head is now 64bfae686... and all commit subjects are conventional (fix(...) / test(...)).
  • Recent runs on the new head are currently action_required (awaiting maintainer approval for fork workflows), not a fresh conventional-commit failure.

Could a maintainer please approve workflows and re-run checks for the latest head commit? Thank you!

Signed-off-by: Phillip <phillipxh@gmail.com>
Copy link
Copy Markdown
Author

Follow-up fix pushed for mobile self-signing:

  • Added a fallback in canSign() when signer me metadata is missing but settings.signerFileUuid is present.
  • Updated RequestSignatureTab.sign() to resolve the sign route UUID robustly (signUuid -> signer sign_uuid -> settings.signerFileUuid -> initial state fallback), and to fail with an explicit error if no UUID is available.
  • Added regression tests for both store-level permission logic and sidebar sign-route UUID fallback.

Local verification:

  • vitest run src/tests/store/files.spec.ts src/tests/components/RightSidebar/RequestSignatureTab.spec.ts
  • Result: 2 passed, 168 passed.

Copy link
Copy Markdown
Member

@vitormattos vitormattos left a comment

Choose a reason for hiding this comment

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

Could you directly into @libresign/pdf-elements ?

@github-project-automation github-project-automation bot moved this from 0. Needs triage to 1. to do in Roadmap Mar 30, 2026
@Phillipxh
Copy link
Copy Markdown
Author

Phillipxh commented Mar 31, 2026

Thanks for the review, that makes sense.

You are right that the touch-placement/runtime workaround should live in @libresign/pdf-elements instead of being implemented in libresign.

I will move the fix into LibreSign/pdf-elements, open a companion PR there, and then update this PR to consume the new package version while keeping only integration-level changes/tests here.

@Phillipxh
Copy link
Copy Markdown
Author

Correction to my previous comment (formatting issue):

Thanks for the review, that makes sense.

You're right that the touch-placement/runtime workaround should live in @libresign/pdf-elements instead of being implemented in libresign.

I will move the fix into LibreSign/pdf-elements, open a companion PR there, and then update this PR to consume the new package version while keeping only integration-level changes/tests here.

@Phillipxh
Copy link
Copy Markdown
Author

Update: I moved the touch-placement fix into @libresign/pdf-elements as requested.\n\nCompanion PR: https://github.com/LibreSign/pdf-elements/pull/49\n\nOnce that PR is merged and released, I can update this libresign PR to consume the new package version and keep only integration-side changes here.

Copy link
Copy Markdown
Member

@vitormattos vitormattos left a comment

Choose a reason for hiding this comment

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

@Phillipxh I published a new release https://github.com/LibreSign/pdf-elements/releases/tag/v1.1.4

Could you go ahead now with the fixed release of pdf-elements package?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 1. to do

Development

Successfully merging this pull request may close these issues.

3 participants