Wp-calypso: Stats - insights: visually broken "no records" message

Created on 8 Dec 2017  ·  6Comments  ·  Source: Automattic/wp-calypso

"No popular day and time recorded" message is visually off.

Steps to reproduce

  1. Starting at URL: https://wordpress.com/stats/insights/:site
  2. For empty / new blog you will see _"No popular day and time recorded"_ message in _"Most popular day and hour"_ box:
    screen shot 2017-12-08 at 20 38 03
  3. Message will break into two lines If you make your browser window just under ~1000px wide or around ~1300px wide:
    screen shot 2017-12-08 at 20 35 10

What I expected

Message box to change its height when text inside it breaks into multiple lines.

What happened instead

The box broke. 😢

Browser / OS version

Firefox / MacOS

☝️ Remember that this was just in English: in other locales there might be more text in this message box.

Design OSS Citizen Stats [Type] Bug good first issue

All 6 comments

Hello all, I would like to tackle this issue. I've been able to replicate the problem, as well as I've been able to come up with a possible fix. I do have a couple of questions, however.

If I change the span's class: .most-popular__notice at the prescribed width and include the CSS:
display:inline-flex

I get the following:
screen shot 2017-12-13 at 10 48 31 am

However, it does appear that where the 'Top' of the box begins gets modified with this specification. This, too, could be adjusted to something like:
Top: 18px
And only be included for the @min-width: 661px display.

This should extend for multiple languages:
screen shot 2017-12-13 at 10 59 12 am

But does beg the question as to best practices, as longer text (beyond two lines) could cause problems. Thoughts?

Tested in FF and Chrome on Mac OSX. Cannot get local copy of Calypso running in Safari.

This is my first attempt at working on anything here - please let me know how I should go about making this update or please let me know if you have any questions or if I'd need to do anything before submitting this merge.

Thanks!

Hey @markpernotto! 👋 So good to see you're interested in fixing this! :-)

I would encourage you to open a PR with instructions how to replicate this (you can copy instructions from this issue), how your fix attempts to fix it and what could be caveats if any. Be mindful about browser compatibility.

I also noticed this issue in Chrome (the box overflowing from right):

screen shot 2017-12-14 at 18 17 32

I can confirm your suggestion fixes that one, too.

I noticed icon is hidden from notifications for small screens at https://wordpress.com/stats/day/:site:
screen shot 2017-12-14 at 19 14 55
screen shot 2017-12-14 at 19 14 49
...perhaps that would work here, too?

But does beg the question as to best practices, as longer text (beyond two lines) could cause problems. Thoughts?

I think your change is quite safe since this CSS class doesn't seem to be re-used anywhere else so there should not be any unintended side-effects.

(FYI @juliaamosova who was the last one modifying this bit of code.)

Hello @simison!

Thanks for the direction. I've tested in all browsers, created my local branch, committed, but I don't seem to be able to send a pull request. I've named the branch: fix/stats-insights-20658, but after each push attempt gets me the message that permission to push is denied.

I've read through all of the documentation, and am sure I'm missing something simple. This would be my first commit to wp-calypso.

Thanks in advance for your assistance.

@markarms Hey! Fantastic you got started! 👏

You should first fork this repo so that you have your own version and push the commit you just made to a branch in your fork.

Then you can create a pull request from your fork back here. In description write "Fixes #20658" thus marking it related to this issue.

You should ensure you're pushing to right repo by checking your remotes, in command line:

git remote -v

I have these two remotes myself:

git remote -v
origin  https://github.com/Automattic/wp-calypso.git (fetch)
origin  https://github.com/Automattic/wp-calypso.git (push)
simison https://github.com/simison/wp-calypso (fetch)
simison https://github.com/simison/wp-calypso (push)

If you need to add your fork as a remote (make sure to fork this repo first), you can:

git remote add markpernotto https://github.com/markpernotto/wp-calypso

Remote's name doesn't need to be "markpernotto" but just as an example. Then you can push to your fork:

git push markpernotto your-branch-name

See also Contributing to Calypso — Pull requests. (Edit, oh sorry, this one you found already :thumbsup:)

Feel free to ask again if you hit any trouble!

Thank you, @simison

I _think_ a pull request has been made. Sincerest appreciation for your detailed direction.

@markpernotto Yep, looks good! It might take some time before someone gets to review that (holidays et all) but hopefully soon. :-)

Congratz for your first ever PR! Well done.

Was this page helpful?
0 / 5 - 0 ratings