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

Update Map and Token Labels to allow users to define how they look #4662

Merged
merged 27 commits into from
Feb 3, 2024

Conversation

cwisniew
Copy link
Member

@cwisniew cwisniew commented Jan 25, 2024

Identify the Bug or Feature request

fixes ##4660

Description of the Change

Token Labels

These are not the default colours, which are closer to the current ones, but I have changed them for these screenshots to make them more obvious. These are all set on the local client's side. (These are not the default colours I just needed something that stood out as being very different when testing)

Preferences Dialog

<img width="257"src="https://github.com/RPTools/maptool/assets/73173/f1d72ad7-1dde-4e20-b2be-ce12cf152d0c">



Map Labels

Labels that are from pre 1.15 campaigns default to have the background colour (well one of them) from the fake 3D "bubble". These can not be changed client side due to the problems

CleanShot 2024-02-03 at 15 16 05@2x CleanShot 2024-02-03 at 15 16 48@2x CleanShot 2024-01-26 at 05 54 50@2x

I have also added comments to the majority of the files I touched that didn't have comments (adding comments to all the methods and fields the renderer and app preferences probably need an issue to address that so I don't completely annoy whom ever will review this change :)), hoping to set a trend with this but doubt it will :)

Possible Drawbacks

Some will miss the fake 3D bubble effect; I am sure someone out there prefers fake 3D bubbles over being able to read and configure the look.

Documentation Notes

See above

Release Notes

  • Added the ability to configure Token labels.
  • Added the ability to configure Map labels.

This change is Reviewable

@cwisniew cwisniew added the feature Adding functionality that adds value label Jan 25, 2024
@FullBleed
Copy link

It looks like there is extra padding on the top of the text boxes.

@cwisniew
Copy link
Member Author

there is extra padding on the top of the text boxes.

Fonts are going to font :(
If you look at the smaller font labels, they are centred (in fact, the smaller one above has a little too much padding at the bottom), but as they scale, you might get more padding at the top or bottom. I fiddled with it a lot but was unable to get it always to line up correctly in every occurance

@FullBleed
Copy link

Fonts are going to font :( If you look at the smaller font labels, they are centred (in fact, the smaller one above has a little too much padding at the bottom), but as they scale, you might get more padding at the top or bottom. I fiddled with it a lot but was unable to get it always to line up correctly in every occurance

Yeah, I see that the smaller the font it starts to drift up and increase the padding below. Seems like a difference between the padding and font sizes calcs. It looks like the fonts are using the point system... would using EM or true pixel size normalize things?

Does running 4k exacerbate the issue?

@cwisniew
Copy link
Member Author

Fonts are going to font :( If you look at the smaller font labels, they are centred (in fact, the smaller one above has a little too much padding at the bottom), but as they scale, you might get more padding at the top or bottom. I fiddled with it a lot but was unable to get it always to line up correctly in every occurance

Yeah, I see that the smaller the font it starts to drift up and increase the padding below. Seems like a difference between the padding and font sizes calcs. It looks like the fonts are using the point system... would using EM or true pixel size normalize things?

Does running 4k exacerbate the issue?

Cant use EM its not web browser based, It's just a case of asking the renderer how big the font is and then using that info to draw the box/centre. It is what it is, trying to do it another way will lead to problems on someones display.

@kwvanderlinde
Copy link
Collaborator

Code changes look good.

As for style, I find the new labels aren't well defined compared to the background, which makes it quite hard for me to focus on them. IMO we should keep a border and a bit of shape with the new flat look, something like this:
image

@FullBleed
Copy link

Code changes look good.

As for style, I find the new labels aren't well defined compared to the background, which makes it quite hard for me to focus on them. IMO we should keep a border and a bit of shape with the new flat look, something like this:

I like that a lot better, too.

@cwisniew
Copy link
Member Author

cwisniew commented Feb 3, 2024

Code changes look good.

As for style, I find the new labels aren't well defined compared to the background, which makes it quite hard for me to focus on them. IMO we should keep a border and a bit of shape with the new flat look, something like this: image

Updated with borders and arcs, both adjustable, can you do another review

@cwisniew cwisniew added this pull request to the merge queue Feb 3, 2024
Merged via the queue into RPTools:develop with commit 4b4b2cf Feb 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants