soduto / Soduto

Soduto is a KDEConnect compatible client for macOS. It allows better integration between your phones, desktops and tablets.
https://www.soduto.com
GNU General Public License v3.0
338 stars 50 forks source link

show battery percentage text #12

Closed Ashinch closed 3 years ago

Ashinch commented 3 years ago

This is a solution that temporarily displays the battery percentage text, maybe we'll find a better display mode in the future.

Preview: preview

Ashinch commented 3 years ago

10

@alexanderadam

alexanderadam commented 3 years ago

I'm not an OSX dev but the screenshot looks great :laughing: Lets see what @giedrius-stanevicius thinks of it.

PS: @Ashinch are you also creating a PR on KDEConnect for the changes that you mentioned?

Ashinch commented 3 years ago

PS: @Ashinch are you also creating a PR on KDEConnect for the changes that you mentioned?

For the pr of KDE-Connect, I need to optimize the code. Although it seems to work normally, but it is currently only a preview version.🤣

giedrius-stanevicius commented 3 years ago

I am still thinking how to separate percentage and device name texts. Maybe add a comma or some other separator, or at least a larger space? What do you guys think about that?

alexanderadam commented 3 years ago

What do you guys think about that?

Personally I like the style of GSConnect where first the device name comes and the percentage in the end. Although they 'cheated' a bit since the font size of the percentage is a bit smaller. But I don't know either what happens when the device name is very long. :wink:

Ashinch commented 3 years ago

@giedrius-stanevicius @alexanderadam

What do you guys think about that?

I think both spaces and · or | are fine.

If they are placed on the right, I don't know what will happen if the device name is too long.🤣

Ashinch commented 3 years ago

@giedrius-stanevicius @alexanderadam

What do you guys think about that?

I think both spaces and · or | are fine.

If they are placed on the right, I don't know what will happen if the device name is too long.🤣

Or we can omit the battery icon and keep only:

percentage-charging icon (if any)-device name

giedrius-stanevicius commented 3 years ago

Or we can omit the battery icon and keep only:

percentage-charging icon (if any)-device name

That could be a solution, but I wonder how it would look like. In general I would like to preserve native look and feel as much as possible and the current icon was designed to mimic the system battery icon (although it is slightly different on Big Sur)

If they are placed on the right, I don't know what will happen if the device name is too long.🤣

This, as I already mentioned earlier, would likely need using custom views. In such case you would have full control over layout constraints, so you could decide how to handle long device names. I think that battery status placement on the right would be ideal, but it would definitely require more effort to implement nicely.

As for the separators, I think that |, · are two graphic and would add more visual noise. I think , is more "texty" and could look more natural.

Overall, I suggest adding , or bigger space first. And if @Ashinch will have time and motivation, right-side placement could be implemented later.

Ashinch commented 3 years ago

Hey ! @giedrius-stanevicius

As for the separators, I think that |, · are two graphic and would add more visual noise. I think , is more "texty" and could look more natural. Overall, I suggest adding , or bigger space first.

I've got three preview screenshots here:

Now it's up to you to choose one for this pr.

And if @Ashinch will have time and motivation, right-side placement could be implemented later.

I read the document you provided. I will finish it later (maybe next month?😂

giedrius-stanevicius commented 3 years ago

I think I like 3rd variant the most. Is precentage in that case part of the icon? I wonder if it looks ok if display scaling is changed.

giedrius-stanevicius commented 3 years ago

I read the document you provided. I will finish it later (maybe next month?😂)

If you happen to try to implement this, one thing to pay attention is keyboard navigation - if it breaks, I am not sure if it worth it then. For example now you can press ^F8 and navigate Soduto menu with keyboard.

Ashinch commented 3 years ago

I wonder if it looks ok if display scaling is changed.

Yes, it works just as well after changing the display scaling. I have amended this pr commit with the modified code.

If you happen to try to implement this, one thing to pay attention is keyboard navigation - if it breaks, I am not sure if it worth it then.

Yes, I found the problem when I tried it yesterday,using custom views can lose keyboard navigation.

giedrius-stanevicius commented 3 years ago

Nice work, @Ashinch! Just one more question before merge. Would you be ok to give away copyrights of this PR code and possibly future contribution to me as an owner of the project or would you rather retain copyrights to yourself? I have no experiance with copyright stuff, but according this (random) article https://haacked.com/archive/2006/01/26/WhoOwnstheCopyrightforAnOpenSourceProject.aspx/ having a single copyright owner in the project could be easier to manage.

Ashinch commented 3 years ago

I don’t need to keep the copyright. It’s a great thing to be able to help this project and @alexanderadam .

giedrius-stanevicius commented 3 years ago

I don’t need to keep the copyright. It’s a great thing to be able to help this project and @alexanderadam .

Thanks!