Use flex for sparkline text instead of block #12466
Conversation
|
Some things don’t look as expected. Will look into that. |
|
Travis did not upload the screenshots to https://builds-artifacts.piwik.org/matomo-org/matomo/3.x-dev/25728/ Can someone trigger the build again or do I need to push something? |
|
@fdellwing That's odd, I restarted the build (https://travis-ci.org/matomo-org/matomo/builds/329917019) |
|
Ok, thats a lot better. But Ecommerce is a problem, but I do not have an matomo instance where I can test this. Does anyone has an instance where I could test this widgets? |
|
HI @fdellwing . Thanks for the PR. Here is my feedback/review:
|
mattab
added
the
Pull Request WIP
This is just a problem in the UI Tests. In "real" the sparklines have the same width as always, I dont know why this happens in the test. Maybe because the placeholder grafic behaves different.
Will look into this. But probably after the weekend as I'm quite busy at work with things I get paid for ;)
Thats a result in using flex instead of block. It is not really visible for the user, but I can fix that if wanted. |
fixxes alignment fixxes UserCountry module
|
Something seems to be broken with Tests, I can not really check what my changes are. It seems to have something to do with #12463. Can you update the screens please and rerun the build? Will look into the results on monday. |
|
@Findus23 Can you restart the build again please? I think the screens got updated. |
|
@fdellwing I think you’ll need to merge the latest 3.x-dev into your branch to fix the tests |
* Fix one system test * Removed totals * Update TotalEvents.php * Changed "right" positioning on ui-dialog hovering buttons (#12458) * Improved Login and Reset Password Fields (#12448) * Improved Login and Reset Password Fields * Update index.twig * Update userSettings.twig * Rename the metrics * Added CIDR Help (#12450) Websites manager, exclude IP addresses, explain that CIDR notation is supported * Added link to make accessibility to Embed guide website easier (#12475) * Added link to make accessibility to Embed guide website * Update index.twig * Rough way of adding the page up and page down shortcuts to help list (#12461) * Rough way of adding page up & down shortcut to help list * To allow Macs to display different shortcut buttons * Improved Print Versions (#12447) * added print, hiding related reports in print version * removed hover from showing in print version. * Improved print version of report to remove next, view visitor profile. Added padding to the top and bottom of own visitor column * Improved print version by removing search, add new page from print. Centered the page counter. * Imroved print version by creating columns in visitor log instead of white space * Removed box shadow from print version when hovered over * Removed totals * Update GetPageTitlesFollowingSiteSearch.php * Update TotalEvents.php * Update Events.php * GeoIP re-attribution: debug output now shows changes to visits geo-location (#12481) Re-create #12478 on the right branch * Changed Feeds URL to HTTPS (#12449) * Changed Feeds URL to HTTPS * Fixed issue if an exception is called with https * Update RssChangelog.php * Fixed 2nd Feed URL * Re alligned icon on metric display (#12464) Align icon in Row Evolution popover * Change Piwik => Matomo in PluginTemplates (#12482) * Small performance improvement in custom tracker (#12443) * Fix a possible issue when there are more than 2 billion visits tracked and system is 32 bits (#12469) as reported in https://forum.matomo.org/t/piwik-log-visit-not-inserting-data-for-high-idvisit-value/26922/3 * Improvements to Annotations listing design (#12476) * Improvements to Annotations listing design :# Please enter the commit message for your changes. Lines starting * fixed create a new annotation Please enter the commit message for your changes. Lines starting * fixed the delete feature so it's not a button * Fixes system tests and a bug * Aligned all icons to left of menus (#12463) * Aligned all icons to left of menus * Minor tweaks for mobile view and horizontal alignment * Modal Button Fix (#12462) Modals popover can now be closed with keyboard * Fixed maintenence mode to show a corresponding message based on record_statistics value (#12479) * Fixed maintenence mode to show a corresponding message based on record_statics value * Update message to "While the maintenance mode is active, data tracking is disabled." * Fixed custom Dimension system tests * Restore wording on Copy dashboard feature (#12483) regressed in #12460 * Fixed selectors hugging left side of browser and search bar icon moving in Responsive View (#12470) * Fixed top controls in Responsive View, fixed search bar icon moving * fix typo * Update layout.less * Shows icon to disable/enable Zen Mode (#12459) * Added icon to enable/disable Zen Mode * Removed comment * Update layout.less * Changed arrow direction and location * Adjusted Zen Mode to correspond to refreshing page * Display issue on dashboard page - double icons * Update zen-mode.js * Update en.json * Update layout.less * Update CoreHome.php * Fix tests (#12488) * update expected system test file * update UI files * update test files * add wait time * update ui files * move site selector loading indicator to the left (#12485) * improve styling of shortcut help (#12486) * improve styling of shortcut help * make them a bit more square * updates expected ui file
|
@mattab the smaller sparklines will be likely the same we experienced with Chromium and may be even a browser bug. |
|
It seems like the rule to get the same align as before is not picked up by the UI tests. div.sparkline script+div {
margin: 1px 0 0 1px;
}Maybe there is no script tag? What should I do? I could give every div inside a sparkline the padding, but that could have some other side effects. Please decide that as you wish. From my side, this PR is finished. The placeholder of the sparklines behaves weird, but it would be an really ugly hack to fix that in UI tests only.
Should I do a clean PR without all with garbage? |
|
Hi @fdellwing - sorry for the delay getting back. Yes, it would be great if you could create a new PR or update this one and only have the few files changed that you propose. We will review it for inclusion then. Thanks! |
|
@fdellwing I wouldn't worry too much about the Sparkline placeholders in the UI tests. In #12066 I am removing them as creating the real Sparklines in the UI tests seems to work. |
|
I did a clean PR for this changes. |
…fdellwing (#12633) * repush #12466 * dont allow the sparkline to shrink * remove unneeded css * Add -ms-flex to sparkline trigger correct display in IE10. * update screenshots
…block" - @fdellwing (matomo-org#12633) * repush matomo-org#12466 * dont allow the sparkline to shrink * remove unneeded css * Add -ms-flex to sparkline trigger correct display in IE10. * update screenshots
Fixes #12451