Skip to content
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

Fix displayed identical fonts in sign PDF #2751

Merged
merged 5 commits into from
Jan 20, 2025

Conversation

omar-ahmed42
Copy link
Contributor

@omar-ahmed42 omar-ahmed42 commented Jan 19, 2025

Description of Changes

Changes:

  • Add a new custom select to display fonts correctly and to allow more styling flexibility in Sign PDF feature by hiding the original <select> element (display: none;) and wrap it with a <div> and then create a custom selection menu using <div>s and CSS to achieve the required results due to the limitations of <select> and <option> while still preserving the hidden <select> for form submission.

Why was the change made?

  1. A bug that caused font families to not be displayed in Firefox.
  2. Select/Option element are not flexible when it comes to styling (compared to DIVs for example) but bullet point 1. is of higher priority.

UI Changes:

  • Dark Mode:

    • Before:
      image

    • After:
      image

  • Light Mode:

    • Before:
      image

    • After:
      image

Note:

  • Changes in sign.js are between the lines 95-228, as it seems the file was auto-formatted affecting whitespaces and single_quotes/double_quotes.

Useful quotes from MDN:

Styling with CSS
Styling the element is highly limited. Options don't inherit the font set on the parent. In Firefox, only color and background-color can be set, however in Chrome and Safari it's not possible to set any properties. You can find more details about styling in our guide to advanced form styling.

Useful references:

Closes #1575


Checklist

General

Documentation

UI Changes (if applicable)

  • Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR)

Testing (if applicable)

  • I have tested my changes locally. Refer to the Testing Guide for more details.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. Bug Something isn't working labels Jan 19, 2025
@github-actions github-actions bot added the Front End Issues or pull requests related to front-end development label Jan 19, 2025
@Frooodle
Copy link
Member

@omar-ahmed42 this was done in older version, but it has issues with firefox and other browsers, does your above changes work on FF?

@omar-ahmed42
Copy link
Contributor Author

omar-ahmed42 commented Jan 19, 2025

@Frooodle Yes, it works now in Firefox but the implementation is different to some degree as I had to hide the original <select> element (display: none;) and wrap it with a <div> and then create a custom selection menu using <div>s and CSS to achieve the required results due to the limitations of <select> and <option>.

Also, the screenshots above are all from Firefox.

Notes:

  1. I tested the changes in Firefox, Edge and Chrome to ensure that they're working well.
  2. I updated the original PR to include the resources and quotes below for easy access for PR readers.

Useful quotes from MDN:

Styling with CSS
Styling the element is highly limited. Options don't inherit the font set on the parent. In Firefox, only color and background-color can be set, however in Chrome and Safari it's not possible to set any properties. You can find more details about styling in our guide to advanced form styling.

Useful references:

@Frooodle
Copy link
Member

Frooodle commented Jan 19, 2025

Do we want to adjust the text displayed with the typed text? Or maintain font name as the text @reecebrowne @omar-ahmed42

Thoughts?

@omar-ahmed42
Copy link
Contributor Author

omar-ahmed42 commented Jan 19, 2025

@Frooodle Personally, I think keeping font names would be better than replacing it with the typed text, why? It makes it easier for the user to memorize and remember the font family used for future use (especially if we had to reorder the font families or if we added more font families).

At first, I thought of displaying a signature preview in a div/container that appears next to the option on hover but I ran into a problem which is "how practical would this solution be for mobile/phone UI?" then I thought of adding a preview field above the font selection to preview the currently selected or hovered over option/font but I can't think of an eye-pleasing design at the moment.

Let me know if you have any ideas

@Frooodle
Copy link
Member

I agree with all of that! Makes sense

@Frooodle Frooodle enabled auto-merge (squash) January 20, 2025 12:09
@Frooodle Frooodle merged commit b4451da into Stirling-Tools:main Jan 20, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Front End Issues or pull requests related to front-end development size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Font Selection Menu Displays Identical Fonts when signing pdfs with text input on firefox
4 participants