Skip to content

Adds Font option to all Sheet type labels (Part 1)#18488

Open
Godmartinz wants to merge 20 commits into
grokability:developfrom
Godmartinz:add-font-option-to-settings
Open

Adds Font option to all Sheet type labels (Part 1)#18488
Godmartinz wants to merge 20 commits into
grokability:developfrom
Godmartinz:add-font-option-to-settings

Conversation

@Godmartinz

Copy link
Copy Markdown
Member

This adds a label font options to all the Sheet type labels. You are able to choose between 4 fonts for your label and data source fonts
image
image

This also corrects the layout of a few labels (L4736_A, L6009_A, L7162_A, L7163_A)

@Godmartinz Godmartinz requested a review from snipe as a code owner January 27, 2026 22:49
@Godmartinz Godmartinz requested review from marcusmoore and snipe and removed request for snipe January 27, 2026 22:50

@marcusmoore marcusmoore left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hm...sizing got weird for me using the default label fields using Sheets\Avery\L7163_A.

Image

Comment thread resources/views/settings/labels.blade.php Outdated
@Godmartinz

Godmartinz commented Jan 28, 2026

Copy link
Copy Markdown
Member Author

Hm...sizing got weird for me using the default label fields using Sheets\Avery\L7163_A.

Image

The field size for L7162's and L7163_A are quite big in comparison to other labels they are 4.6mm-5mm per field. vs .15mm per field on the other sheets. I know we agreed to not scale the labels, but they look very tiny next to these field values. 😅

@marcusmoore

Copy link
Copy Markdown
Collaborator

I see what you're saying...

L7163_A on develop for example:
image

Putting the label and data source on the same line for that and L7162's might not be the right move 🤔

@Godmartinz

Godmartinz commented Jan 28, 2026

Copy link
Copy Markdown
Member Author
image

made some size adjustments to the field and title, looks better now.

@marcusmoore

Copy link
Copy Markdown
Collaborator

I think L7162_A needs the same treatment as L7163_A:

image

😅

@marcusmoore

Copy link
Copy Markdown
Collaborator

You're already on it 💪🏾

@Godmartinz

Copy link
Copy Markdown
Member Author
image

@snipe

snipe commented Jan 28, 2026

Copy link
Copy Markdown
Member

Has this been checked with LTR and CJK?

@Godmartinz

Godmartinz commented Jan 28, 2026

Copy link
Copy Markdown
Member Author

It works 🙂 :
image

it looks like in the writeText() method, it overrides any font if the characters are cjk with 'cid0cs' .
'font_family' => Helper::isCjk($text) ? 'cid0cs' : $fontFamily,

@Godmartinz

Godmartinz commented Jan 28, 2026

Copy link
Copy Markdown
Member Author

looks like there is an issue for RTL langs. working on it

@Godmartinz

Godmartinz commented Jan 28, 2026

Copy link
Copy Markdown
Member Author

RTL languages dont seem to work on develop either 🤔

@marcusmoore marcusmoore left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me. The only other suggestion I have is wondering if the fonts should be in a constant somewhere to avoid using strings all over the place but it's not a big deal in this case.

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.

3 participants