Use vector icons for nearby-map markers #621 #786

akaita · 2017-07-16T22:50:33Z

As described in the issue #621, replacing pngs like this:

With this svg:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg
   xmlns:dc="http://purl.org/dc/elements/1.1/"
   xmlns:cc="http://creativecommons.org/ns#"
   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
   xmlns:svg="http://www.w3.org/2000/svg"
   xmlns="http://www.w3.org/2000/svg"
   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
   height="28"
   viewBox="0 0 24 28"
   width="24"
   version="1.1"
   id="svg3839"
   sodipodi:docname="custom_marker_akaita.svg"
   style="fill:#000000"
   inkscape:version="0.92.1 r">
  <metadata
     id="metadata3845">
    <rdf:RDF>
      <cc:Work
         rdf:about="">
        <dc:format>image/svg+xml</dc:format>
        <dc:type
           rdf:resource="http://purl.org/dc/dcmitype/StillImage" />
        <dc:title></dc:title>
      </cc:Work>
    </rdf:RDF>
  </metadata>
  <defs
     id="defs3843" />
  <sodipodi:namedview
     pagecolor="#ffffff"
     bordercolor="#666666"
     borderopacity="1"
     objecttolerance="10"
     gridtolerance="10"
     guidetolerance="10"
     inkscape:pageopacity="0"
     inkscape:pageshadow="2"
     inkscape:window-width="1680"
     inkscape:window-height="925"
     id="namedview3841"
     showgrid="true"
     inkscape:zoom="24.375"
     inkscape:cx="5.1897436"
     inkscape:cy="14.875286"
     inkscape:window-x="0"
     inkscape:window-y="23"
     inkscape:window-maximized="0"
     inkscape:current-layer="svg3839"
     showguides="false">
    <inkscape:grid
       type="xygrid"
       id="grid3904" />
  </sodipodi:namedview>
  <ellipse
     style="fill:#000000;fill-opacity:0.06666667;stroke-width:1.37026358"
     id="path3899"
     cx="12.102563"
     cy="22.428205"
     rx="6.0307693"
     ry="3.6717949" />
  <path
     d="m 12,2.3769234 c -3.8699996,0 -6.9999996,3.13 -6.9999996,7 C 5.0000004,14.626923 12,22.376923 12,22.376923 c 0,0 7,-7.75 7,-12.9999996 0,-3.87 -3.13,-7 -7,-7 z m 0,9.4999996 c -1.38,0 -2.4999996,-1.12 -2.4999996,-2.4999996 0,-1.38 1.1199996,-2.5 2.4999996,-2.5 1.38,0 2.5,1.12 2.5,2.5 0,1.3799996 -1.12,2.4999996 -2.5,2.4999996 z"
     id="path3835"
     inkscape:connector-curvature="0" />
  <path
     d="M 0,4 H 24 V 28 H 0 Z"
     id="path3837"
     inkscape:connector-curvature="0"
     style="fill:none" />
  <path
     style="fill:#f84d4d;fill-opacity:1;stroke-width:0.04102564"
     d="M 11.575408,11.825361 C 10.688745,11.666747 9.9023831,10.964562 9.6248346,10.08359 9.5527876,9.8549054 9.5351956,9.7044904 9.5384636,9.3451284 c 0.00361,-0.396913 0.019229,-0.491787 0.1296458,-0.787492 0.2355178,-0.630736 0.6458016,-1.098647 1.2116146,-1.381792 0.386227,-0.193277 0.708521,-0.271742 1.116173,-0.271742 0.676431,0 1.262546,0.246849 1.7436,0.734337 0.354734,0.359479 0.541139,0.681663 0.657408,1.136269 0.32693,1.2782826 -0.441524,2.6108316 -1.722787,2.9874286 -0.281992,0.08289 -0.816778,0.113659 -1.09871,0.06322 z"
     id="path3849"
     inkscape:connector-curvature="0" />
  <path
     style="fill:#006699;fill-opacity:1;stroke:#003b59;stroke-width:1;stroke-miterlimit:4;stroke-dasharray:none;stroke-opacity:1"
     d="M 11.616794,21.911674 C 10.518479,20.62925 9.3375979,19.068762 8.3952457,17.654529 6.5241437,14.846476 5.4546019,12.509947 5.1020419,10.460171 5.014133,9.9490711 5.0064946,8.8329764 5.0875486,8.3424774 5.3480263,6.7661894 6.0432736,5.4266055 7.1582617,4.3526805 9.1483483,2.4358857 12.016137,1.8727748 14.59345,2.8927283 c 2.043205,0.808584 3.606586,2.5813031 4.161658,4.7189081 0.174081,0.670391 0.204374,0.932573 0.20343,1.760678 -9.22e-4,0.8100256 -0.03496,1.0975936 -0.21977,1.8566086 -0.61444,2.523538 -2.571462,5.977389 -5.383242,9.500614 -0.645397,0.808696 -1.321301,1.610028 -1.358021,1.610028 -0.0078,0 -0.179158,-0.192551 -0.380711,-0.427891 z m 1.000543,-10.103061 c 0.78285,-0.188213 1.457153,-0.795278 1.737658,-1.564386 0.516156,-1.4152346 -0.316651,-2.9618196 -1.783469,-3.3120326 -0.216428,-0.05167 -0.31719,-0.05889 -0.661455,-0.04738 -0.35432,0.01185 -0.441293,0.02512 -0.681724,0.103976 -0.673464,0.220894 -1.205333,0.695479 -1.5059061,1.343717 -0.1763343,0.380295 -0.2182632,0.584137 -0.2168632,1.054307 9.661e-4,0.323935 0.013916,0.452389 0.063992,0.6346746 0.2664071,0.969774 1.0768523,1.68863 2.0794153,1.844418 0.243193,0.03779 0.680462,0.01192 0.968352,-0.0573 z"
     id="path3859"
     inkscape:connector-curvature="0" />
</svg>

Which looks like this:

Drawer before the change (Nexus 5x)	Drawer after the change (Nexus 5x)

misaochan · 2017-07-18T07:23:53Z

This looks good to me, but I will ping @neslihanturan for input first.

misaochan · 2017-07-20T08:31:47Z

@neslihanturan I am hoping to merge this soon, if no issues from your end.

neslihanturan · 2017-07-20T09:52:13Z

It looks very nice, sorry for delay of mine. I think we should carry all of our png's to svg in near future. There is some failed tests, should we ignore them?

misaochan · 2017-07-20T10:26:51Z

At this stage, unfortunately I never really know if that indicates an issue with the test, with the PR, or with Travis (Travis has given us a lot of false negatives). Regardless, I will restart the build and hopefully we'll find out!

akaita · 2017-07-20T13:30:43Z

@neslihanturan great!
@misaochan the tests in Travis seem to fail because some Lint rules like RtlHardcoded are being run as fatal rules... not sure why.
The codacy rules... well it complains about the format of the javadoc, because the first line is /**... I don't really agree with codacy :)

misaochan · 2017-07-20T13:44:45Z

@whym , what do you think is the issue with the tests? The error messages seem rather vague to me.

Thanks for your patience @akaita !

whym · 2017-07-20T14:30:04Z

app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java

@@ -98,21 +100,24 @@ public int compare(Place lhs, Place rhs) {

        placeList = placeList.subList(0, Math.min(placeList.size(), MAX_RESULTS));

+        Bitmap icon = UiUtils.getBitmap(
+                VectorDrawableCompat.create(


This line appears to cause a connected test error when called by NearbyControllerTest:

android.content.res.Resources$NotFoundException: Resource ID #0x7f02005d at android.content.res.ResourcesImpl.getValue(ResourcesImpl.java:190) at android.content.res.Resources.getDrawable(Resources.java:766) at android.support.v4.content.res.ResourcesCompatApi21.getDrawable(ResourcesCompatApi21.java:31) at android.support.v4.content.res.ResourcesCompat.getDrawable(ResourcesCompat.java:60) at android.support.graphics.drawable.VectorDrawableCompat.create(VectorDrawableCompat.java:579) at fr.free.nrw.commons.nearby.NearbyController.loadAttractionsFromLocationToBaseMarkerOptions(NearbyController.java:104) at fr.free.nrw.commons.NearbyControllerTest.testEmptyList(NearbyControllerTest.java:49)

You can run connected tests with "gradle connectedCheck" after launching an emulator.

@tobias47n9e I believe you wrote NearbyControllerTest. Perhaps could you figure out what is happening here?

whym · 2017-07-20T14:30:42Z

Regarding the codacy results, please try adding a period after "Draws a vectorial image onto a bitmap". (It says "First sentence of Javadoc is incomplete (period is missing) or not present.")

Regarding tests, I have commented in line.

akaita · 2017-07-20T15:06:16Z

Oh, my bad. I admit I didn't run the tests. I didn't change any method, so I assumed it would be fine... :(

Also fixed the codacy issue. Thanks for clearing that up for me @whym !

codecov-io · 2017-07-20T15:21:51Z

Codecov Report

❗ No coverage uploaded for pull request base (master@44d294e). Click here to learn what that means.
The diff coverage is 75%.

@@           Coverage Diff            @@
##             master    #786   +/-   ##
========================================
  Coverage          ?   6.75%           
========================================
  Files             ?     100           
  Lines             ?    5151           
  Branches          ?     468           
========================================
  Hits              ?     348           
  Misses            ?    4776           
  Partials          ?      27

Impacted Files	Coverage Δ
...a/fr/free/nrw/commons/nearby/NearbyController.java	`25% <60%> (ø)`
...c/main/java/fr/free/nrw/commons/utils/UiUtils.java	`85.71% <85.71%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44d294e...1ddf8be. Read the comment docs.

akaita added 2 commits July 16, 2017 23:40

Replace low-def bitmap with svg

2a0047d

Tidy up following codacy's suggestions

8a28654

commons-app deleted a comment Jul 16, 2017

misaochan requested a review from neslihanturan July 18, 2017 07:23

whym reviewed Jul 20, 2017

View reviewed changes

akaita added 2 commits July 20, 2017 15:50

Make codacy happy

046a2f9

Fix connected tests

1ddf8be

commons-app deleted a comment Jul 20, 2017

whym merged commit e9d77a0 into commons-app:master Jul 22, 2017

whym mentioned this pull request Jul 22, 2017

Use vector icons for nearby-map markers #621

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use vector icons for nearby-map markers #621 #786

Use vector icons for nearby-map markers #621 #786

akaita commented Jul 16, 2017

misaochan commented Jul 18, 2017

misaochan commented Jul 20, 2017

neslihanturan commented Jul 20, 2017 •

edited

Loading

misaochan commented Jul 20, 2017

akaita commented Jul 20, 2017

misaochan commented Jul 20, 2017 •

edited

Loading

whym Jul 20, 2017 •

edited

Loading

whym Jul 20, 2017

whym commented Jul 20, 2017

akaita commented Jul 20, 2017

codecov-io commented Jul 20, 2017

Use vector icons for nearby-map markers #621 #786

Use vector icons for nearby-map markers #621 #786

Conversation

akaita commented Jul 16, 2017

misaochan commented Jul 18, 2017

misaochan commented Jul 20, 2017

neslihanturan commented Jul 20, 2017 • edited Loading

misaochan commented Jul 20, 2017

akaita commented Jul 20, 2017

misaochan commented Jul 20, 2017 • edited Loading

whym Jul 20, 2017 • edited Loading

Choose a reason for hiding this comment

whym Jul 20, 2017

Choose a reason for hiding this comment

whym commented Jul 20, 2017

akaita commented Jul 20, 2017

codecov-io commented Jul 20, 2017

Codecov Report

neslihanturan commented Jul 20, 2017 •

edited

Loading

misaochan commented Jul 20, 2017 •

edited

Loading

whym Jul 20, 2017 •

edited

Loading