Giter Site home page Giter Site logo

Comments (20)

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024 1
1. Can `WatchUi.CustomMenuItem`s be added to a `WatchUi.Menu2`, or must they be added to a `WatchUi.CustomMenu`? Which means all menu items would then need to be converted to `WatchUi.CustomMenuItem`.

CustomMenuItem inherits from WatchUi.MenuItem, so should work with Menu2 too.

2. Even if we do use `WatchUi.CustomMenu`, how do we provide the required `:drawable` option? We can create a Drawable, but what size does it need? I can't see where to get sizings from for the constructor.

My first guess would be, that Graphics.DC (device context) would be the thing for this.

Starting to think this is going to be troublesome.

Yep, my proposal would be, that using IconMenuItem would be good enough for a "MVP". And maybe, someday, when we think, we need a more fancy design, we could considering using CustomMenuItem :)

from garminhomeassistant.

JosephAbbey avatar JosephAbbey commented on September 12, 2024 1

Hello,

I removed the redundant code from the spike and aligned the naming of the menu item types. I would say, the "lean ui" branch is ready for a code review and after that, we could do the pull request.

https://github.com/Someone0nEarth/GarminHomeAssistant/tree/lean_ui

Have fun,
someone

Hi, could you create a pull request now because it makes code review easier and you can continue to commit to your branch (and the pull request) once it is created.

Thanks

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

I have to agree, the second line of text has not been adding much value. A good idea at the time...
I quite like this proposal, preferring the "kebab" to the "hamburger" icon mainly because of consuming the width needed for text.
I also think making an option in the UI makes the suggestion a no brainer.

Thank you for your interest.

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

Hello!

I am already writing on a prototype/spike for this feature and being close to finish it. First time doing something with Monkey C... so my progress is a bit slow ;-)

When it is ready to show, I will post the branch.

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

We've so far used SVG for icons from here in order to be able to scale them with a Python script to all the sizes required for the different models of watch. We'll then need to modify that script to work with non-square icons in order to maintain the aspect ratio.

Look forward to hearing from you.

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

Here is the current state of the spike:

https://github.com/Someone0nEarth/GarminHomeAssistant/tree/lean_ui

Its on "make it work" level. For "make it right" it would need to remove code redundancy, adjust icons positions, tune the resize script and so on.

Using Monkey C, especially the connect IQ Api is quite a adventure... for example IconMenuItem layout behaves differently to ToggleMenuItem: For Toggle, the icon can be right of the label and the label is centered. For Icon this is not possible (the label will be right aligned). Maybe a CustomMenuItem would be a solution for a more fitting design.

My next step will be to clean up the code à la "make it right", as a small learning session, what are the capabilities of Monkey C.

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

I'll have a go at buliding it locally this afternoon, I can see the work you've put in.

You may have noted from the Python script, we use the 48x48 SVG icons as the source and amend the sizes from there to make the other folders. Fixing the icon script should be trivial.

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

Yep, I used 48x48 as the source ✌️

At the moment, the two new SVGs are white (because the app has a black background on my Venu), but on the emulator (Linux), the background is white, so they are not visible on that (at least for me). I didn`t do adjustment of the icon size, positioning etc. yet.

I would like to see the label centered and the icon on the right side (like the layout of the toggle menu item). My current impression is, that using CustomMenuItem for this, is the only way to go. But as already mentioned: First time ever for me coding Garmin stuff :)

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

I'm trying to understand:

  1. Can WatchUi.CustomMenuItems be added to a WatchUi.Menu2, or must they be added to a WatchUi.CustomMenu? Which means all menu items would then need to be converted to WatchUi.CustomMenuItem.
  2. Even if we do use WatchUi.CustomMenu, how do we provide the required :drawable option? We can create a Drawable, but what size does it need? I can't see where to get sizings from for the constructor.
  3. Can we use a resource file to define the labels we want customise in the CustomMenuItem? The documentation for this is scant/obtuse, and there do not seem to be any working examples to copy.

Starting to think this is going to be troublesome. BTW I'm still very much learning "Monkey C" too. I did a little development a while back on a Garmin controller for a Google Nest, but that understanding has now evapourated! (The app was not release for several complicating reasons.)

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

If we change everything to be left aligned, the the text only goes under the icon when it is too wide for the space.
image

And another difference between emulator and real watch is that the text size scales on the watch, so it looks better on the real device.

I think the IconMenuItem still has potential.

If we reconsider the icon width required, then the text should align, or perhaps this is just another detail that is not consistent between emulator and real device?

image

(Red icons only so I could see them on a white background.)

If you agree, then we'll just need to factor out the HTTP GET/POST code into a base class to be inherited by both subtext and leanUI classes.

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

CustomMenuItem inherits from WatchUi.MenuItem, so should work with Menu2 too.

Except I was unable to get the CustomMenuItem.draw() function to be executed. I did get a blank entry in a WatchUi.Menu2. So I was checking for reasons.

My first guess would be, that Graphics.DC (device context) would be the thing for this.

Graphics.DC:

"You should never directly instantiate a Dc object, or attempt to render to the screen outside of an onUpdate call."

I think we need a WatchUi.Drawable for this. I've just not figured out how to specify settings parameter with :height and :width.

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

Heyhey again!

I like the idea with the left alignment! Will test it on my device.

I think the IconMenuItem still has potential.
👍

If we reconsider the icon width required, then the text should align, or perhaps this is just another detail that is not consistent between emulator and real device?
As far as I understand is, that the icon space and the label space is in the general layout when using IconMenuItem "disjunct" (on real devices. Yep, looks like, that the emulator is
inaccurate.. again).

If you agree, then we'll just need to factor out the HTTP GET/POST code into a base class to be inherited by both subtext and leanUI classes.

Yes, dealing with the code redundancy was although on my todo-list :)

I will change the lean ui to left aligment and do the refactorings ✌️

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

I was wondering if for simplicity the non-lean UI should go left aligned for consistency? It was only ever right aligned as that was the default and I did not change it. No good reason for the choice, other than MVP code.

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

I was wondering if for simplicity the non-lean UI should go left aligned for consistency? It was only ever right aligned as that was the default and I did not change it. No good reason for the choice, other than MVP code.

I added an app setting for the alignment (which is used for lean and sublabel).

Sadly.. ToggleItems labels are still centered, while the IconItems label are left/right aligned. This is especially a little annoying, when the menu has toogle and icon items.. but for a MVP it is fine enough for me. And maybe one day Garmin will align the behavior of both item types.

from garminhomeassistant.

philipabbey avatar philipabbey commented on September 12, 2024

When I simulated the venu2 above, it looked likt ToggleItems obeyed the alignment.

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

I don`t trust the emulators any longer ;) On my (real) Venu, the Toggle labels are centered while the icon labels are left (right) aligned :/

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

After I removed the code redundancy, I would say the feature would be ready for a pull request.

I changed the tap-icon color to blue and the menu-icon color to grey. I am open for other suggestions :)

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

CustomMenuItem inherits from WatchUi.MenuItem, so should work with Menu2 too.

Except I was unable to get the CustomMenuItem.draw() function to be executed. I did get a blank entry in a WatchUi.Menu2. So I was checking for reasons.

Looks like you are right: The API doc says for for example for ToggleMenuItems "A ToggleMenuItem can be added to a Menu2 using the addItem() method." and for CustomMenuItem "A CustomMenuItem can be added to a CustomMenu using the addItem() method.".

But yeah.. maybe a topic for the future!

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

Hello,

I removed the redundant code from the spike and aligned the naming of the menu item types. I would say, the "lean ui" branch is ready for a code review and after that, we could do the pull request.

https://github.com/Someone0nEarth/GarminHomeAssistant/tree/lean_ui

Have fun,
someone

from garminhomeassistant.

Someone0nEarth avatar Someone0nEarth commented on September 12, 2024

I synced the feature branch with main and rebased it.

from garminhomeassistant.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.