Skip to content

Conversation

@mruss77
Copy link
Contributor

@mruss77 mruss77 commented Oct 26, 2021

The "status bar" that appears above the quick settings & app menu was directly coded into those screens. Developers wanting to add a status bar to the top of their apps would need to copy the code and get a reference to the battery controller and datetime controller.

This PR moves the code to draw the status bar into DisplayApp. Apps that want a status bar just need to declare a struct (DisplayApp::StatusBar) and call the methods to draw the status bar and to update it periodically.

Another benefit of this approach is that the status bar can be updated globally in one place. I updated it to show 12-hour or 24-hour time based on the time format setting.

I updated the app menu code in ApplicationList and Tile and the QuickSettings code to use this new functionality to draw the status bar.

Not many apps use the status bar currently, but I think it would be useful to see the current time when setting the alarm.

I used the StatusBar struct to hold the pointers for the LVGL objects in the status bar, so that the screen actually displaying them would own them and they wouldn't go out of scope... not sure if this is the best approach, but it works.

@mruss77
Copy link
Contributor Author

mruss77 commented Oct 26, 2021

Forgot to mention, I added a convenience class "FormatTime" for converting between 12- and 24-hour time formats.

@Riksu9000
Copy link
Contributor

In the applist the battery icon touches the pageindicator. According to https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/doc/ui_guidelines.md#ui-design-guidelines the icons should move 8px to make space for the page indicator.

Maybe the statusbar should be a new class instead of being a part of DisplayApp, or a part of the Screen class. What do you think?

Other than that this seems good for now. Further improvements can be done afterwards.

@mruss77
Copy link
Contributor Author

mruss77 commented Oct 27, 2021

@Riksu9000 - Thanks, had not noticed the problem with the page indicator.

Would it make sense to just move the battery indicator 8px left (regardless of whether there's a page indicator)? This would break the first UI guideline (always align to the edge or corner), but it would keep the battery icon from appearing to move around on different screens. If it needs to be shifted only when there's a page indicator, I guess the calling app would have to pass a parameter to indicate whether the page indicator is there.

About where this code belongs, I don't have a strong preference... I chose DisplayApp mainly because it already has a reference to the datetime and battery controllers. If this becomes its own class, would it have to be initialized in SystemTask and passed a reference to these controllers?

As a separate question, I was wondering whether systemtask should provide methods to get a handle to the various controllers at runtime, instead of having to pass them in when the system boots... But that's pretty far out of scope of this PR.

@Riksu9000
Copy link
Contributor

Would it make sense to just move the battery indicator 8px left (regardless of whether there's a page indicator)? This would break the first UI guideline (always align to the edge or corner), but it would keep the battery icon from appearing to move around on different screens.

It would have to be moved in quicksettings, digital and analog watchfaces. It might be noticable if it was aligned differently, which wouldn't be great. Moving the icons 8px when there's a page indicator isn't great either though. I haven't experimented with other solutions yet, but I would like to try for example shrinking the page indicator instead, but for now I think it would be best to keep the UI as it is and look at improving that separately.

If this becomes its own class, would it have to be initialized in SystemTask and passed a reference to these controllers?

It could just be initialized in DisplayApp or right inside the app, which would require passing all the controllers to the app. Actually maybe DisplayApp should be the one responsible for drawing overlay type things, which aren't necessarily a part of the running app. It could still be a separate class though just to keep the different components neatly organized.

@Riksu9000
Copy link
Contributor

We might need more time to figure out what to do about the status bars, so could you create another PR with just the time format changes, so we can get that merged quicker?

@mruss77
Copy link
Contributor Author

mruss77 commented Nov 10, 2021

@Riksu9000 sure, will do that today

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