GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||
GeezerVison is showing github.com — Pride flag watchface by Aperture32GLaDOS · Pull Request #2201 · InfiniTimeOrg/InfiniTime Conversation
|
|
Build checks have not completed. Possible reasons for this are:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution :)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thanks for the quick feedback! I've implemented those changes, hopefully everything's good now |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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: 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." Like I said, very very small suggestions. The other 99% of this watchface is grand! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…m "Trans" to "Trans Flag"
There was a problem 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 😁
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Good spot, I'll fix that |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 😁 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 😜) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
could you update to/rebase onto the latest |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Yup, should be all good now |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
totally optional: could you update the initial PR description to contain updated images for the currently implemented pride flags? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
suggestions to make the build work again
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
- Implemented all suggested changes
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
… UseFlagData to private
There was an error while loading. Please reload this page.
There was a problem 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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Yeah, no worries - I'm flashing it now |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I don't think it's a big issue - I'll leave whether white or black works better for the trans face to you :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
In that case, I'd prefer to leave it as-is, with black for the time and white for everything else |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem 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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…constant variables const, and generally tidying up)
There was an error while loading. Please reload this page.
There was a problem 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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem 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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Brilliant! Thanks all for the advice and feedback, it's been very helpful |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.