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

DRY-ing text related codes #55

Open
martonmiklos opened this issue Dec 11, 2024 · 2 comments
Open

DRY-ing text related codes #55

martonmiklos opened this issue Dec 11, 2024 · 2 comments

Comments

@martonmiklos
Copy link
Contributor

I came across a formatting issue with an InputNumber (underline was not present).

The reason is the fact that the font flags are not handled here:
https://github.com/Open-Agriculture/AgIsoVirtualTerminal/blob/main/src/InputNumberComponent.cpp#L47

At the input strings they are handled:
https://github.com/Open-Agriculture/AgIsoVirtualTerminal/blob/main/src/InputStringComponent.cpp#L42

I would like to avoid code duplication so first I thought that I creating a common function for converting the FontAttribute and the Juce::Font would be the way to go. But after further peeking the code creating a TextDrawingComponent and a NumberComponent would look like a better idea, as there are many code pieces which are redundant (text alignment, etc.)

What do you think about this approach?

@ad3154
Copy link
Member

ad3154 commented Dec 12, 2024

One reason they are not combined right now is because they inherit from isobus::OutputNumber vs isobus::InputNumber for example, which allows for convenient use of isobus::VTObject::get_object_type and other things like that. In theory I'm not opposed to combining them but there will probably be other things that need to get re-done to make that happen, or at the very least you may need to override some virtual functions in creative ways

@martonmiklos
Copy link
Contributor Author

One reason they are not combined right now is because they inherit from isobus::OutputNumber vs isobus::InputNumber for example, which allows for convenient use of isobus::VTObject::get_object_type and other things like that. In theory I'm not opposed to combining them but there will probably be other things that need to get re-done to make that happen, or at the very least you may need to override some virtual functions in creative ways

Thanks for the answer, I will sort it out and get back for review.

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

No branches or pull requests

2 participants