Improve look of Pokémon marker label by daangroot · Pull Request #2476 · RocketMap/RocketMap

daangroot · 2018-02-01T20:51:38Z

Description

Small improvement of style and formatting of the Pokémon marker label.
See screenshots below for the changes made.

Motivation and Context

I did not really like the look of the label, so I thought lets make it more beautiful :)

How Has This Been Tested?

Tested on my main setup.

Screenshots (if appropriate):

Old:

New:

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.

darkelement1987 · 2018-02-02T16:14:08Z

I like it! The way it shows IV's is much better like this.

jaake77

I like it. Works.

ghost · 2018-02-02T20:29:06Z

I like this, minimal and easier to read, how would it account for the recent PR #2475 (adding gen info to marker labels), possibly next to level?

sebastienvercammen

Increase top margin on the link for the coordinates. People on mobile + fat fingers = accidents.

Alderon86 · 2018-02-02T21:03:09Z

@daangroot add margin-top: 5px; to the # nav part to make the changes @sebastienvercammen suggested for fat fingers.
Without Iv:

With Iv:

daangroot · 2018-02-02T21:05:37Z

@sebastiaanvercammen I don't see why a top margin would help. There is nothing above it that could be pressed, so nothing can go wrong.

Alderon86 · 2018-02-02T21:07:31Z

@daangroot u could press something like exclude or notify for e.g.

daangroot · 2018-02-02T21:11:55Z

Oh yes sorry, did not think about the no iv label.

daangroot · 2018-02-02T22:52:55Z

I added a 5px top margin to the navigate class as suggested.

Alderon86 · 2018-02-03T22:37:47Z

  }

  &.navigate {
+    position: absolute;


why do we need that? im not a fan of position: absolute.

That's how it was done before. It can be done differently but this is the easiest way.

Alderon86 · 2018-02-03T22:40:18Z

    height: 48px;
    width: 48px;
-    margin-bottom: 11px;
+    margin-bottom: 15px;


Is there a reason why we would have to increase it?
Also, can u provide a screenshot with the difference compared to my simple change?

To align the links on the left with the location link. I think it looks better this way.

dripdropper · 2018-02-04T08:59:54Z

You should consider leaving the ADS labels on the IVs to eliminate confusion. Many (not all) tools use ADS, but the in-game appraisal uses SAD.
Otherwise it looks like good, especially the size reduction, which helps to keep things on-screen. It's too bad the Google marker API is too stupid to place the InfoWindow in a direction that keeps it on-screen.

daangroot · 2018-02-04T10:26:58Z

@dripdropper I removed them because it looked better, but I can understand that it might cause confusion, so I will add them back.

fosJoddie · 2018-02-05T20:45:25Z

nice changes, looks neat and triggers no warning or epileptic episodes

jaake77 · 2018-02-06T14:41:00Z

Werks Gud. Thanks for your efforts.

sirdemoncze · 2018-02-06T18:30:22Z

Thanks. It works perfect 👍.

Alderon86

👍 good work

billyjbryant · 2018-02-11T16:37:08Z

Looks good and works.

sebastienvercammen

Final change, rest looks OK. 👍

sebastienvercammen · 2018-02-14T17:46:03Z


 function getTypeSpan(type) {
-    return `<span style='padding: 2px 5px; text-transform: uppercase; color: white; margin-right: 2px; border-radius: 4px; font-size: 0.6em; vertical-align: middle; background-color: ${type['color']}'>${type['type']}</span>`
+    return `<span style='padding: 2px 5px; text-transform: uppercase; color: white; margin-right: 2px; border-radius: 4px; font-size: 0.75em; vertical-align: middle; background-color: ${type['color']}'>${type['type']}</span>`


Move all the inline CSS to the stylesheet, and remove the getTypeSpan function. Since the HTML will be much shorter, it'll be clearer if it's no longer a separate function.

daangroot · 2018-02-16T23:03:44Z

Most up-to-date look of the label:

sebastienvercammen

RE: Discord:

[11:57 PM] Konturka: There is a small issue though. The type is not centered inside its box on some mobile browsers.
[11:58 PM] Konturka: This was already an issue but it was less noticeable because the font size was smaller.(edited)
[12:00 AM] Konturka: On chrome and samsungs browser it's not centered.
[12:00 AM] Konturka: On firefox it is.

billyjbryant · 2018-02-19T04:28:42Z

Looking at the Type box code and testing locally

billyjbryant · 2018-02-19T06:39:18Z

Adding padding-top: 0.45em (or padding-top:2.25%) on the .type element fixes it for mobile browsers but breaks on desktop. this could probably be fixed by adding the necessary overrides to mobile.scss

daangroot · 2018-02-24T15:39:30Z

@sebastienvercammen I've tried a lot of things to make the type text centered, but I couldn't fix it.

daangroot · 2018-02-24T15:40:04Z

Maybe we can merge it and fix it afterwards. I can make an issue for this problem, so other people are aware of it and can maybe fix it.

sebastienvercammen · 2018-02-27T13:13:15Z

@daangroot We won't merge without the fix. The PR is instead tagged as "help wanted".

daangroot · 2018-03-22T18:08:34Z

Hopefully the final look:

sebastienvercammen added front-end awaiting review labels Feb 1, 2018

sebastienvercammen added this to the 4.1.1 milestone Feb 1, 2018

jaake77 approved these changes Feb 2, 2018

View reviewed changes

sebastienvercammen requested changes Feb 2, 2018

View reviewed changes

Alderon86 approved these changes Feb 2, 2018

View reviewed changes

daangroot closed this Feb 3, 2018

daangroot reopened this Feb 3, 2018

darkelement1987 approved these changes Feb 3, 2018

View reviewed changes

Alderon86 suggested changes Feb 3, 2018

View reviewed changes

fosJoddie approved these changes Feb 5, 2018

View reviewed changes

darkelement1987 approved these changes Feb 6, 2018

View reviewed changes

Alderon86 approved these changes Feb 6, 2018

View reviewed changes

daangroot added 4 commits February 8, 2018 21:56

Improved pokemon label contentstring

5e6ed7f

Change css style

9ef73fb

More style improvements.

7200949

Fix travis and change margin to margin-bottom.

f7ae73a

daangroot added 2 commits February 8, 2018 21:56

Make the location link fat finger friendly.

6bb1da3

Add attack/defense/stamina labels to iv values.

c6eee6c

daangroot force-pushed the label-update branch from cbb97e5 to c6eee6c Compare February 8, 2018 20:58

billyjbryant approved these changes Feb 11, 2018

View reviewed changes

Alderon86 approved these changes Feb 14, 2018

View reviewed changes

sebastienvercammen requested changes Feb 14, 2018

View reviewed changes

Move type CSS to stylesheet and remove getTypeSpan function.

fe9cc8d

michikrug mentioned this pull request Feb 16, 2018

Make RM look nice #2496

Open

6 tasks

sebastienvercammen reviewed Feb 18, 2018

View reviewed changes

sebastienvercammen added help wanted pending changes and removed awaiting review labels Feb 27, 2018

daangroot added 5 commits March 1, 2018 15:22

Merge branch 'develop' into label-update

942ff68

Merge remote-tracking branch 'origin/develop' into label-update

a4f4791

Merge remote-tracking branch 'upstream/develop' into label-update

2ad8ed7

Merge remote-tracking branch 'upstream/develop' into label-update

6f64650

Increase font size to make type center.

c24fa18

Conversation

daangroot commented Feb 1, 2018

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

darkelement1987 commented Feb 2, 2018

Uh oh!

jaake77 left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 2, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebastienvercammen left a comment

Choose a reason for hiding this comment

Uh oh!

Alderon86 commented Feb 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daangroot commented Feb 2, 2018

Uh oh!

Alderon86 commented Feb 2, 2018

Uh oh!

daangroot commented Feb 2, 2018

Uh oh!

daangroot commented Feb 2, 2018

Uh oh!

Alderon86 Feb 3, 2018

Choose a reason for hiding this comment

Uh oh!

daangroot Feb 4, 2018

Choose a reason for hiding this comment

Uh oh!

Alderon86 Feb 3, 2018

Choose a reason for hiding this comment

Uh oh!

daangroot Feb 4, 2018

Choose a reason for hiding this comment

Uh oh!

dripdropper commented Feb 4, 2018

Uh oh!

daangroot commented Feb 4, 2018

Uh oh!

fosJoddie commented Feb 5, 2018

Uh oh!

jaake77 commented Feb 6, 2018

Uh oh!

sirdemoncze commented Feb 6, 2018

Uh oh!

Alderon86 left a comment

Choose a reason for hiding this comment

Uh oh!

billyjbryant commented Feb 11, 2018

Uh oh!

sebastienvercammen left a comment

Choose a reason for hiding this comment

Uh oh!

sebastienvercammen Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

daangroot commented Feb 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebastienvercammen left a comment

Choose a reason for hiding this comment

Uh oh!

billyjbryant commented Feb 19, 2018

Uh oh!

billyjbryant commented Feb 19, 2018

Uh oh!

daangroot commented Feb 24, 2018

Uh oh!

daangroot commented Feb 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebastienvercammen commented Feb 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

ghost commented Feb 2, 2018 •

edited by ghost

Loading

Alderon86 commented Feb 2, 2018 •

edited

Loading

daangroot commented Feb 16, 2018 •

edited

Loading

daangroot commented Feb 24, 2018 •

edited

Loading

sebastienvercammen commented Feb 27, 2018 •

edited

Loading

daangroot commented Mar 22, 2018 •

edited

Loading