Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make statistics menu less sad - decorations and colour #4203

Merged
merged 43 commits into from
Oct 26, 2022
Merged

Make statistics menu less sad - decorations and colour #4203

merged 43 commits into from
Oct 26, 2022

Conversation

matkoniecz
Copy link
Member

fixes #3414

screen05

example with extra placeholder to demonstrate styling

screen01
screen02
screen03
screen04

@matkoniecz
proof of concept for placing laurel leaf
72e6acd
@matkoniecz
extra code with some unexpected failures
8c2db12
@matkoniecz
Merge branch 'master' into laureat
ee0d534
@matkoniecz
proof of concept of nonterrible laurel wreaths
bb49e68
@matkoniecz
first laurel wreath that is actually nice
d3f8157
@matkoniecz
abandoned night mode attempt
6140ad1
@matkoniecz
Revert "abandoned night mode attempt"
f4f1abe 
This reverts commit 6140ad1.
@matkoniecz
Merge branch 'master' into laureat
04ff843
@matkoniecz
ktlint terror
5b2535a
@matkoniecz
try to fix leading leaf offset on left side
03c21f7
@matkoniecz
uniformify drawable sizes so rotations will be predictable
2b117ff
@matkoniecz
shorten stalk on leaf ending to eliminate artefact
c49444f
@matkoniecz
more stringent testing of renderings
3cea77e
@matkoniecz
drop dead code and fix typo
27e73b7
@matkoniecz
start applying styling to real ranks
7f2585a
@matkoniecz
avoid merging decoration with text
9a61547 
could happen with for example ranking where long position such as #10000 may qualify for noticeable decoration
@matkoniecz
more encouraging scaling of active days for new mappers
3117e03
@matkoniecz
remove test graphics
409ef51
@matkoniecz
tweak colors for background
fc049b1
@matkoniecz
tweak colors backround ranking bubbles
8cbecf0 
to make text more visible on them
@matkoniecz
Merge branch 'master' into laureat
fb415ab
@matkoniecz
note authorship
8bf90d2
@matkoniecz
make placeholders more clear
64eb57a
@matkoniecz
ktlint terror
aca6ef3
@matkoniecz
drop misleading comment
a0dc92a
@matkoniecz
further comment fixes
412435c
@matkoniecz
minor code refactoring
5528e67
@matkoniecz
explain code, tiny moving to more logical order
c3220a6
@matkoniecz
tweak reward scaling
90434fb
@matkoniecz
better describe what is going on
79861bb
@westnordost
Copy link
Member

westnordost commented Jul 12, 2022 •
edited
Loading

  1. Could you put the laurel wreath class into an own file in /view?
  2. The drawing of the stalk is needlessly complicated (and looks a bit odd because there are so many corners). To draw an arc is actually really simple, see https://developer.android.com/reference/android/graphics/Canvas#drawArc(float,%20float,%20float,%20float,%20float,%20float,%20boolean,%20android.graphics.Paint). But actually, better than that would be if you just skipped the stalk and used a different drawable for the leaf, for example something like this: https://commons.wikimedia.org/wiki/File:Olivenkranz.png (or this or this or this)

@matkoniecz
Copy link
Member Author

For now I applied drawArc

screen07

westnordost
westnordost reviewed Jul 14, 2022
View reviewed changes
app/src/main/java/de/westnordost/streetcomplete/view/LaurelWreath.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/view/LaurelWreath.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/view/LaurelWreath.kt Outdated
val canvasHeight: Int = bounds.height()
val circleRadius: Float = Math.min(canvasWidth, canvasHeight).toFloat() / 2f

val niceSubtleGreen: Paint = Paint().apply { setARGB(255, 152, 184, 126) }
Copy link
Member

Choose a reason for hiding this comment

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

should be a private field

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you sure that it should be moved out of that function?

Copy link
Member

@westnordost westnordost Oct 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I am just parroting what the IDE warns about. Android Studio warns about creation of objects in the draw function for performance reasons. Since this paint is always the same, it also makes sense.

It's weird that your Android studio didn't display a warning there? Did you switch off warnings?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's weird that your Android studio didn't display a warning there? Did you switch off warnings?

I switched off some that were consistently annoying and useless. I can enable them again, and triage them again - maybe they improved something.

app/src/main/java/de/westnordost/streetcomplete/view/LaurelWreath.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/view/LaurelWreath.kt Outdated Show resolved Hide resolved
app/src/main/java/de/westnordost/streetcomplete/view/LaurelWreath.kt Outdated
}

// right side
val flippedBitmap = bitmap.flipHorizontally()
Copy link
Member

Choose a reason for hiding this comment

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

FYI you can also apply a matrix transformation (to flip the bitmap) on drawing on the canvas, might be easier.

https://developer.android.com/reference/android/graphics/Canvas#drawBitmap(android.graphics.Bitmap,%20android.graphics.Matrix,%20android.graphics.Paint)

(But you are not exactly drawing thousands of wreaths each frame, so perfomance of creating dozens of intermediate bitmaps is no issue)

westnordost
westnordost reviewed Oct 25, 2022
View reviewed changes
Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

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

Looks good, one small remark

app/src/main/java/de/westnordost/streetcomplete/screens/user/profile/ProfileFragment.kt Show resolved Hide resolved
@westnordost westnordost merged commit e11eea6 into streetcomplete:master Oct 26, 2022
@matkoniecz matkoniecz deleted the laureat branch October 26, 2022 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@westnordost westnordost westnordost approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make statistics menu less sad - more interesting display of data (colours? patterns?)
2 participants
@matkoniecz @westnordost