Skip to content

Conversation

@Aperture32GLaDOS
Copy link
Contributor

@Aperture32GLaDOS Aperture32GLaDOS commented Dec 16, 2024

Adds a new watch face with the trans flag as a background

Features

  • A new watch face, featuring the mlm, trans, bi, and wlw flags
  • Bluetooth indicator in the top right
  • Long day names

Images

image
image
image
image

@github-actions
Copy link

github-actions bot commented Dec 16, 2024

Build checks have not completed. Possible reasons for this are:

  1. The checks need to be approved by a maintainer
  2. The branch has conflicts
  3. The firmware build has failed

@mark9064 mark9064 added the new watchface This thread is about a new watchface label Dec 22, 2024
@marigoldfish
Copy link

marigoldfish commented Dec 30, 2024

Love it! 🏳️‍⚧️😁 You are a very cool person, I'm going to be using this for a while!

Edit to add: I hope you find $20 on the ground or something similar fortunate and nice happens to you!

IMG_20241229_213604426~2

Copy link
Member

@mark9064 mark9064 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution :)

@Aperture32GLaDOS
Copy link
Contributor Author

Thanks for the quick feedback! I've implemented those changes, hopefully everything's good now

@mark9064
Copy link
Member

mark9064 commented Jan 3, 2025

All looks good to me! You couldn't have made review easier :)

I haven't had the chance to test on hardware yet and am away for now, but I should be able to test within a couple weeks time

@mark9064 mark9064 added this to the 1.16.0 milestone Jan 3, 2025
@marigoldfish
Copy link

I have been using this daily, and I really like it, but I have a few small suggestions:

I adore the "You have mail." for notifications, but I dislike that it overlaps so severely with the battery percentage:

IMG_20241231_082654043~2

I think it might look better to have "mail." on its own line below, and move "You have" up a little bit so they're both centered vertically in the blue stripe.

(The 90's kid in me really wants it to read "You've got mail," like AOL, but that's not a request for a change, just a nostalgic observation)

My other suggestion is to name the watch face more specifically as "Trans Flag" instead of just "Trans."

IMG_20250103_220544316_HDR~2

Like I said, very very small suggestions. The other 99% of this watchface is grand!

@Aperture32GLaDOS
Copy link
Contributor Author

I definitely agree with the notification and the name. Fortunately, these are just one-line fixes so it's not that hard to change.
image

Copy link

@marigoldfish marigoldfish left a comment

Choose a reason for hiding this comment

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

If I can nitpick, I think "Mail" should be lowercase. But this looks really good 😁

@Aperture32GLaDOS
Copy link
Contributor Author

Good spot, I'll fix that

@marigoldfish
Copy link

You are an incredibly awesome and speedy person! Thanks for implementing my suggestions, and so fast! I'll keep testing this daily for a while and let you know if I come across anything else 😁

@marigoldfish
Copy link

Hello again, I noticed another little thing after using this watch face for a while.

Usually I keep my watch in 24 hour time, but I had reason to use 12 hour time today, and I noticed that both AM and PM show up as "M."

(I manually set the watch to 2am to get the second picture, I didn't stay up late 😜)

IMG_20250117_184147295~2
IMG_20250117_184236513~3

@Aperture32GLaDOS
Copy link
Contributor Author

I think what's happening here is that the font used for the time doesn't have all the alphabet, if I change the font to something else it seems to work fine:
image
I've moved the AM/PM display to the day, since that uses a font which has all the alphabet
image

@marigoldfish
Copy link

That's an elegant fix! I like it!

Unfortunately, in addition to praise I also come bearing news of a new bug: it is displaying 24-hour time when I have 12-hour time enabled.

IMG_20250119_203002810~2

@Aperture32GLaDOS
Copy link
Contributor Author

Aah, well at least that's an easy fix; it should all be working now
image

@NeroBurner
Copy link
Contributor

could you update to/rebase onto the latest main branch. The toolchain won't run as the artifacts v3 job was deprecated by GitHub, and that is fixen in the current main branch

@Aperture32GLaDOS
Copy link
Contributor Author

Yup, should be all good now

@rdub1a4
Copy link

rdub1a4 commented Feb 10, 2025

Seems like this would wait for side loading. It may not fit the vision of "Only a minimal feature set in the flashed firmware". But, no biggy.

@JustScott
Copy link
Contributor

JustScott commented Feb 10, 2025

Seems like this would wait for side loading. It may not fit the vision of "Only a minimal feature set in the flashed firmware". But, no biggy.

I dont understand why this watchface was added to the 1.16.0 milestone so quickly over the various other watchface PRs that have been sitting around for quite a long time?

@mark9064
Copy link
Member

mark9064 commented Feb 10, 2025

Mainly because it doesn't depend on any other PRs and was easy to review

Things get added to the milestone when we want to target including them this cycle. If a PR depends on lots of other refactoring for example, or has outstanding bugs that are taking time to resolve, then it usually won't be queued up for the next cycle until it's ready. InfiniTime generally lacks reviewers though, so what gets queued and what doesn't depends on whether someone's had time to look at it properly

Edit: also you're totally welcome to help with review or express support/interest in PRs in the comments, it helps with prioritising review

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

coming along nicely. Some code duplication I'd like to get rid of remains

@NeroBurner
Copy link
Contributor

totally optional: could you update the initial PR description to contain updated images for the currently implemented pride flags?

Copy link
Member

@mark9064 mark9064 left a comment

Choose a reason for hiding this comment

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

Looking good! Sounds like my thoughts are pretty similar to @NeroBurner regarding duplication

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

suggestions to make the build work again

Copy link
Member

@mark9064 mark9064 left a comment

Choose a reason for hiding this comment

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

I think this is good to go!! The flags will never look right in AOD with only 8 colours but I'd like to make sure text at least stays readable. Would you be able to test it quickly? If not I can flash mine

@Aperture32GLaDOS
Copy link
Contributor Author

Yeah, no worries - I'm flashing it now

@Aperture32GLaDOS
Copy link
Contributor Author

wlw
bi
mlm
trans
The time is legible on all of them, but on the trans watch the day and date are invisible - if it's an issue we could change all text to be black

@mark9064
Copy link
Member

I don't think it's a big issue - I'll leave whether white or black works better for the trans face to you :)

@Aperture32GLaDOS
Copy link
Contributor Author

In that case, I'd prefer to leave it as-is, with black for the time and white for everything else

@mark9064 mark9064 requested a review from NeroBurner June 17, 2025 15:31
Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

much constness, much wow 🤓

I found a way to have PrideFlagData in cpp file. See comments

…constant variables const, and generally tidying up)
Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

just one last const, and then I think we're good to go!

It's been lovely working with you on this MR @Aperture32GLaDOS

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

all looking good. Thanks for the contribution and your great teamwork! I had fun reviewing and learning on this PR

@Aperture32GLaDOS
Copy link
Contributor Author

Brilliant! Thanks all for the advice and feedback, it's been very helpful

@mark9064 mark9064 merged commit 4517fb8 into InfiniTimeOrg:main Jun 19, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new watchface This thread is about a new watchface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants