-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(bar): add bar's orientation #6212
base: master
Are you sure you want to change the base?
Conversation
- add bar' orientation feature
- add feature bar orientation
- add test_case for bar orientation
- as astylerc format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #6172
The test took me a moment to understand but it seems reliable 😁
src/widgets/bar/lv_bar.h
Outdated
@@ -109,6 +120,13 @@ void lv_bar_set_range(lv_obj_t * obj, int32_t min, int32_t max); | |||
*/ | |||
void lv_bar_set_mode(lv_obj_t * obj, lv_bar_mode_t mode); | |||
|
|||
/** | |||
* Set the type of bar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated docstring
* Set the type of bar. | |
* Set the orientation of the bar. |
src/widgets/bar/lv_bar.h
Outdated
@@ -148,6 +166,13 @@ int32_t lv_bar_get_max_value(const lv_obj_t * obj); | |||
*/ | |||
lv_bar_mode_t lv_bar_get_mode(lv_obj_t * obj); | |||
|
|||
/** | |||
* Get the type of bar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for reviewing and checking the wording.
- change comment type --> orientation for orientation APIs
src/widgets/bar/lv_bar.h
Outdated
@@ -58,6 +68,7 @@ typedef struct { | |||
_lv_bar_anim_t cur_value_anim; | |||
_lv_bar_anim_t start_value_anim; | |||
lv_bar_mode_t mode : 2; /**< Type of bar*/ | |||
lv_bar_orientation_t orient : 2; /**< Orientation of bar*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use full name: orientation
instead of shortened name orient
(see: clean code -> "avoid mental mapping")
In C / C++ languages using full names instead of shortenings does not influence code behaviour and code size, but makes code easier to read. No need to figure out what names mean ("mental mapping")
/** | ||
* Set the orientation of bar. | ||
* @param obj pointer to bar object | ||
* @param orient bar orientation from ::lv_bar_orientation_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could write full parameter name: orientation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree!
@@ -399,4 +403,92 @@ void test_bar_render_corner(void) | |||
render_test_screen_create(true, LV_GRAD_DIR_VER, "widgets/bar_corner_6.png"); | |||
} | |||
|
|||
void test_bar_orientation(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add reference image also to compare the result with that.
Please simplify the code in this test, it just needs to test lv_bar_set_orientation
function, and no need for special style for bar. Except maybe: add gradient color to see that orientation really works as expected. Similar gradient as mentioned here: #6172 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with these too.
orient->orientation
orient -> orientation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for opening this PR. Please fix the last two things pointed out @PGNetHun and I think it can be merged.
/** | ||
* Set the orientation of bar. | ||
* @param obj pointer to bar object | ||
* @param orient bar orientation from ::lv_bar_orientation_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree!
@@ -399,4 +403,92 @@ void test_bar_render_corner(void) | |||
render_test_screen_create(true, LV_GRAD_DIR_VER, "widgets/bar_corner_6.png"); | |||
} | |||
|
|||
void test_bar_orientation(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with these too.
@TridentTD Do you have time to finalize this PR? |
Description of the feature or fix
Add feature bar's orientation.
Notes
lv_conf_template.h
run lv_conf_internal_gen.py and update Kconfig.scripts/code-format.py
(astyle version v3.4.12 needs to be installed) and follow the Code Conventions.