search

fury-gl / fury

FURY - Free Unified Rendering in pYthon.
https://fury.gl
Other
241 stars 181 forks source link

Fixing `TextBlock2D` justification issue #790

Closed ganimtron-10 closed 1 year ago

ganimtron-10 commented 1 year ago

Fixing the issue of alignment in TextBlock2D when using the justification and vertical_justification property as we can see below. image image

This was happening because when we were updating the text justification it was with the reference of the current position of the UI element which is the bottom left corner of the element.

The fix proposed solve this issue as shown below.

image image

ganimtron-10 commented 1 year ago

Before creating the tests, I would like to know whether this approach of setting the justification in the add_to_scene method would be valid? or do I have to plan out some other approach to obtain this?

@skoudoro @guaje

skoudoro commented 1 year ago

I would like to know whether this approach of setting the justification in the add_to_scene method would be valid?

It is not valid. add_to_scene should have the minimum code possible. So, you should avoid to put code there. Also, as I said on my comment above, justification should stay in place if we move /resize an UI.

it would be great if you could fix this before friday. Thanks @ganimtron-10 !

skoudoro commented 1 year ago

small note, please, take the habit to add labels and reviewers in your PRs

ganimtron-10 commented 1 year ago

It is not valid. add_to_scene should have the minimum code possible. So, you should avoid to put code there. Also, as I said on my comment above, justification should stay in place if we move /resize an UI.

But as I mentioned I get the size of the background only when the UI element is added to the scene so I need to do the required changes after adding to the scene. You get a good idea on Friday after testing.

small note, please, take the habit to add labels and reviewers in your PRs

I was unable to add @JoaoDell and @tvcastillod as reviewers and was waiting for their review so didn't added you to the same. Will do this from next PR onwards.

skoudoro commented 1 year ago

From the previous meeting, you can either update this PR or close it and create a new PR.

boundingbox need to be implemented. see our taskboard.

Thank you @ganimtron-10

ganimtron-10 commented 1 year ago

Thanks for the update @skoudoro. Will create a new PR.