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.

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

8075 labels in graph overlaps #8078

Merged
merged 6 commits into from Jun 18, 2015
Merged

8075 labels in graph overlaps #8078

merged 6 commits into from Jun 18, 2015

Conversation

sebastianpiskorski
Copy link
Contributor

Added code for overlapping labels removal
Fixed UI test setViewportSize() function

Solution for Issue #8075

@tsteur
Copy link
Member

tsteur commented Jun 9, 2015

Can you append a screenshot how the added UI test label_ticks_cutout looks after that change?

@tsteur
Copy link
Member

tsteur commented Jun 9, 2015

I can't really say much do that as I'm not really into jqplot etc. From my side it would be ok to merge if it works and if it is well tested from your side. Have you also tested it in different browsers and resolutions?

@mattab mattab added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Jun 10, 2015
@mattab mattab added this to the 2.14.0 milestone Jun 10, 2015
@sebastianpiskorski
Copy link
Contributor Author

@tsteur Here are screenshots. Before:
evolutiongraph_annotations_none

and after:
evolutiongraph_ticks_cutout

@tsteur
Copy link
Member

tsteur commented Jun 10, 2015

How did it look before when the widget was that small?

@sebastianpiskorski
Copy link
Contributor Author

@tsteur Here you have:

evolutiongraph_label_ticks_cutout

Tick were cut because i gave them 5px gutter from each side, so they are more readable.

@tsteur
Copy link
Member

tsteur commented Jun 11, 2015

Cool, that looks good :) Which browsers have you tested?

@sebastianpiskorski
Copy link
Contributor Author

@tsteur
Firefox 38.05
Chrome 43.0
Safari 8.0

@tsteur
Copy link
Member

tsteur commented Jun 14, 2015

Do you mind testing in IE before merging if possible? IE 8 and IE10 or so would be great otherwise just IE9 or IE10 should do for now. Let me know if it is possible

@sebastianpiskorski
Copy link
Contributor Author

@tsteur I work on mac and I don't have access to IE currently. Either in office nor in home. Maybe if I could install some VM, but I can't say when I will be able to do it.

@tsteur
Copy link
Member

tsteur commented Jun 15, 2015

You might be interested in http://dev.modern.ie/tools/vms/ :)

@sebastianpiskorski
Copy link
Contributor Author

@tsteur
Copy link
Member

tsteur commented Jun 17, 2015

I use VirtualBox on MacOSX. Works fine :)

@sebastianpiskorski
Copy link
Contributor Author

@tsteur Here are screenshots from IE 9,10,11on Win7, Oracle VirtualBox (ietestdrive)
screen shot 2015-06-18 at 7 03 28 pm
screen shot 2015-06-18 at 7 11 36 pm
screen shot 2015-06-18 at 7 26 27 pm

@tsteur
Copy link
Member

tsteur commented Jun 18, 2015

Cheers!

tsteur added a commit that referenced this pull request Jun 18, 2015
@tsteur tsteur merged commit c98e8de into matomo-org:master Jun 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants