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

Add 5 min display timeout #79

Merged
merged 2 commits into from
Apr 16, 2024
Merged

Conversation

kevdliu
Copy link
Contributor

@kevdliu kevdliu commented Apr 15, 2024

This PR adds a 5 minute option to the list of display timeout durations. 10 minutes is a bit long (in my opinion) since most of the time I'm only making short adjustments.

src/sysinfo_panel.cpp Outdated Show resolved Hide resolved
@ballaswag
Copy link
Owner

I think you actually want to add new options at the very end, with the next index instead of reordering them. The options text can be ordered, but not the mapped indexes. Existing settings will be surprised if their sleep values are changed.

@kevdliu
Copy link
Contributor Author

kevdliu commented Apr 16, 2024

Thanks for the feedback. I'm actually having trouble understanding why the current ordering wouldn't work. Looking at this code

lv_dropdown_set_options(display_sleep_dd,
			"Never\n"
                        "5 Minutes\n"
			"10 Minutes\n"
			"30 Minutes\n"
			"1 Hour\n"
			"5 Hours");

if (!v.is_null()) {
    auto sleep_sec = v.template get<int32_t>();
    const auto &el = sleepsec_to_dd_idx.find(sleep_sec);
    if (el != sleepsec_to_dd_idx.end()) {
      lv_dropdown_set_selected(display_sleep_dd, el->second);
    }
  }

The menu looks up the index of the selected option using sleepsec_to_dd_idx, so as long as the mapping in sleepsec_to_dd_idx is consistent with the list passed to lv_dropdown_set_options then everything should still work right?

@ballaswag
Copy link
Owner

You are right! I had the key value reversed in my head. The store value is actually the second and not the index.

Thanks for taking the time to adjust the nonsense.

@kevdliu
Copy link
Contributor Author

kevdliu commented Apr 16, 2024

You are right! I had the key value reversed in my head. The store value is actually the second and not the index.

Thanks for taking the time to adjust the nonsense.

No worries! Glad I'm not just dumb hahah.

@ballaswag ballaswag merged commit fa6a286 into ballaswag:main Apr 16, 2024
0 of 4 checks passed
@kevdliu kevdliu deleted the display_sleep branch April 16, 2024 18:11
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.

2 participants