andrivet / ADVi3pp

ADVi3++, an alternative and better firmware for Wanhao i3 Plus printers and clones. Fork of Marlin Firmware.
https://community.advi3pp.com
GNU General Public License v3.0
243 stars 119 forks source link

LCD UI elements consistency adjustments #347

Closed taligentx closed 2 months ago

taligentx commented 2 months ago

Thanks for the extensive work to bring current versions of Marlin and new features to these printers! This PR is to address some LCD UI elements in the 5.8.0 release that seem to be inconsistent:

Description

LCD-Panel/DGUS-root/TFT:

LCD-Panel/Masters/ADVi3++5 LCD-Panel.sketch

Requirements

5.8.0

Benefits

Adjusts LCD UI elements for consistency.

Configurations

n/a

Related Issues

n/a

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

andrivet commented 2 months ago

Thanks for the PR. I will review it in the next few days.

andrivet commented 2 months ago

Unfortunately, it is not technically possible to merge this PR. Binary files are not merge-friendly. So instead, I have manually made some of the modifications you proposed:

022_main: Bed current temp font size does not match other temps, increased font size. Fixed

036_preheat: fan speed font color does not match fan speed in other screens, changed font color. Fixed

084_setup_no_sensor: displays X-Twist setup but not a valid selection for no sensor firmware, removed.

Fixed

At least as of Sketch.app 100.3, some element widths are too small and overflow to two lines (°C symbol, Temperature labels, plus others). Interestingly, the same elements render correctly in Lunacy but the updated sketch file renders correctly in both apps. Not sure of the problem, so this is not fixed.

Sensor Settings: Y-axis (-) pressed icon not set to pressed.

Fixed

Setup No Sensor - Shows 2. X-Twist but not used in no sensor firmware, removed.

Fixed

Widgets/LED/Output - marked as exportable but not used for firmware, unset exportable.

This is indeed used by ADVi3++ so not modified

Main: Bumped version number on screenshot

Used only in the user manual, it is already bumped to 6.0.0

I do not plan to backport those changes to 5.8.0, so they will be available with version 6.0.0.

taligentx commented 2 months ago

Unfortunately, it is not technically possible to merge this PR. Binary files are not merge-friendly. So instead, I have manually made some of the modifications you proposed:

Good to hear about the fixes, and noted about the binary files.

At least as of Sketch.app 100.3, some element widths are too small and overflow to two lines (°C symbol, Temperature labels, plus others). Interestingly, the same elements render correctly in Lunacy but the updated sketch file renders correctly in both apps. Not sure of the problem, so this is not fixed.

I'll grab a few screenshots and can follow this up in an Issue.

Widgets/LED/Output - marked as exportable but not used for firmware, unset exportable.

This is indeed used by ADVi3++ so not modified

As above, I'll follow this up in an Issue.

Main: Bumped version number on screenshot

Used only in the user manual, it is already bumped to 6.0.0

I do not plan to backport those changes to 5.8.0, so they will be available with version 6.0.0.

Sounds good, looking forward to it!

andrivet commented 2 months ago

Are you registered on ADVi3++ Community? If yes, can send me a private message there? Thans in advance.