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

feat: Added custom theme builder #161

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Austin-Fulbright
Copy link

This was created because of a feature request here #110

This theme builder is not fully complete which is why I am opening this as a draft. The theme builder will allow you to build a custom theme by entering RGB codes into the create custom theme window. Your custom theme will be one of the selections of possible themes.

This code is not super optimized yet and I hope it doesn't stray to far away from regular conventions. I would love feed back if you could offer any.

:)

@Austin-Fulbright
Copy link
Author

Here is a demo of how it works currently:

IMG_1616.mov

@Austin-Fulbright Austin-Fulbright changed the title added custom theme builder feat: Added custom theme builder Sep 11, 2024
@Austin-Fulbright
Copy link
Author

Also sorry for forgetting to include my .gitignore for the .vscode extension. I will add it next commit.

@Austin-Fulbright
Copy link
Author

I am actually going to configure this. I want to be able to do a rgb color panel like this: https://www.w3schools.com/colors/colors_picker.asp

@greenaddress
Copy link
Contributor

nice work, just two comments:

dependencies.lock.esp32s3 shouldn't be in .gitignore, at least in the PR.
and unless you are changing idf, idf_components.yml dependencies.lock.esp32s3 nor dependencies.lock.esp32 should change in a PR



void generate_color_string(char* buffer, size_t size, const char* color_name, int color_value) {
snprintf(buffer, size, "%s{%d}", color_name, color_value);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually 'const' int params where possible.
We usually JADE_ASSERT pointers and sizes/length params to functions.
We usually JADE_ASSERT the result of snprintf calls.

snprintf(buffer, size, "%s{%d}", color_name, color_value);
}

void handle_color_activity(const char* color_name, int* color, gui_view_node_t* color_text, gui_activity_t* parent_act) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert comments as above

int32_t ev_id;
while (gui_activity_wait_event(act, GUI_EVENT, ESP_EVENT_ANY_ID, NULL, &ev_id, NULL, 0)) {
if (ev_id == GUI_WHEEL_LEFT_EVENT) {
*color = (*color - 8 + 256) % 256;
Copy link
Collaborator

@JamieDriver JamieDriver Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe have a 'const uint16_t limit = UINT8_MAX + 1' rather than 256 or a #define ?
Maybe #define the '8' step size also.

// Update color string and repaint
generate_color_string(color_str, sizeof(color_str), color_name, *color);
gui_update_text(color_c_text, color_str);
gui_repaint(color_c_text->parent);
Copy link
Collaborator

@JamieDriver JamieDriver Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gui_repaint() shouldn't be needed - it should do this itself.

@JamieDriver
Copy link
Collaborator

JamieDriver commented Sep 26, 2024

Nice idea! Some small comments in the code.

Some thoughts (just ideas!):

  • A simple entry could use the pin entry screen to enter an rgb value (we'd have to tweak pin-entry screen so it had a 0-F hex mode as well as the existing 1-9 decimal mode. May not work, just an idea ...

  • some sort of feedback of the final colour while you're editing the values, or after the value has been entered, with 'confirm', 'go back to editing the colour', and 'cancel' options ?

  • maybe change the selected theme to 'custom' mode if the user confirms the colour - that's probably what they want...

  • It might be worth making 'gui_flags' a uint32_t and storing any theme colour in the top half. We'd have to see which was better use of nvs space. (There is a small wrinkle in doing this, see storage_get_qr_flags())

Just to be clear, the above are just musings/ideas, not hard 'you must...' comments. ;-)

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

Successfully merging this pull request may close these issues.

3 participants