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.

Sign up for GitHub

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

Jump to bottom

New design for alerts and notifications #7874

Merged
merged 1 commit into from May 12, 2015
Merged

New design for alerts and notifications #7874

merged 1 commit into from May 12, 2015

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented May 10, 2015

#7585

The new design was proposed by Rafa's mockup for the installer, updater and admin screens (example here): I have applied the same consistent style everywhere.

I have also simplified Less rules as much as possible: UI notifications now use alert CSS classes. To make things clear:

  • alert CSS classes allow to create alert blocks anywhere in the page (e.g. used in the installer, updater, admin screens, etc.), these are part of the content and are static
  • notifications are used using JS or the PHP api and are displayed dynamically at the top of the page to notify the user of something: notifications are currently styled using alert CSS classes (which could change in the future)

Screenshot:

capture d ecran 2015-05-11 a 07 51 25

Code:

<div class="alert alert-info">...</div>
<div class="alert alert-success">...</div>
<div class="alert alert-warning">...</div>
<div class="alert alert-danger">...</div>

Those are based on Bootstrap alerts (but FYI we are still not using Bootstrap).

The alerts currently lack the icons, the new icon set is under development.

@mnapoli mnapoli added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. c: Design / UI For issues that impact Matomo's user interface or the design overall. labels May 10, 2015
@mnapoli mnapoli added this to the 2.14.0 milestone May 10, 2015
@mnapoli mnapoli added the Needs Review PRs that need a code review label May 10, 2015
@mnapoli mnapoli mentioned this pull request May 10, 2015
Merged
@mnapoli
Copy link
Contributor Author

mnapoli commented May 10, 2015

The UI screenshot diffs are here: http://builds-artifacts.piwik.org/ui-tests.redesign-alerts/12385.7/screenshot-diffs/diffviewer.html

The diff for the demo page is here

@diosmosis
Copy link
Member

Looks good to me. I guess have to wait until the icons are done, then it can be merged.

@mnapoli
Copy link
Contributor Author

mnapoli commented May 10, 2015

I don't think we need to wait for the icons, they are needed everywhere else in Piwik too. Some alerts were already redesigned in the last release (e.g. the updater) even though we don't have the icons. They can come later (and by the way previously alerts didn't have any icons anyway).

By the way maybe let's give some time to others to see the PR before merging just in case? (~1 day?)

@diosmosis
Copy link
Member

No worries, let me know if you want to merge whenever it's ready.

tsteur
tsteur reviewed May 11, 2015
View reviewed changes
plugins/Morpheus/stylesheets/less/_variables.less
@@ -23,6 +23,10 @@
@theme-color-widget-title-text: @theme-color-text;
@theme-color-widget-title-background: @theme-color-background-tinyContrast;

@theme-color-success: #60AD61;
@theme-color-warning: #DF9D27;
@theme-color-danger: #E0645D;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should put this kinda stuff as well to a advanced.less or _colors.less in #7876

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@tsteur tsteur mentioned this pull request May 11, 2015
Open
@tsteur
Copy link
Member

tsteur commented May 11, 2015

Are the colors like this wanted/defined by Rafa? The contrast seems to be a bit low and it's a bit hard to read. Or is it just me?

@tsteur
Copy link
Member

tsteur commented May 11, 2015

Eg in this screenshot shouldn't it rather use an error color? http://builds-artifacts.piwik.org/ui-tests.redesign-alerts/12385.7/processed-ui-screenshots/Installation_db_setup_fail.png

@mnapoli
Copy link
Contributor Author

mnapoli commented May 11, 2015

In the screenshot you linked to it's the form that uses the warning class, not error. We could fix that though (I have a branch WIP for forms redesign).

The error (red) + info (grey) were picked from the updater redesign, the rest need to be fixed with the latest screenshots (didn't have them at the time I did this change). Will change the colors using the latest screenshots!

@mnapoli
Copy link
Contributor Author

mnapoli commented May 12, 2015

Colors updated, variables removed for now, ready for merge:

capture d ecran 2015-05-12 a 22 15 38

@tsteur
Copy link
Member

tsteur commented May 12, 2015

I still find the contrast not ideal (still hard to read for me) but it might be just my eyes or screen. otherwise 👍

mnapoli added a commit that referenced this pull request May 12, 2015
@mnapoli
Merge pull request #7874 from piwik/redesign-alerts
1073691 
New design for alerts and notifications
@mnapoli mnapoli merged commit 1073691 into master May 12, 2015
@mnapoli mnapoli deleted the redesign-alerts branch May 12, 2015 21:41
@mattab
Copy link
Member

mattab commented May 13, 2015

In the screenshot you linked to it's the form that uses the warning class, not error. We could fix that though

👍

maybe the warning color has not enough contrast... we can adjust later after seeing it in action

@mattab mattab mentioned this pull request May 20, 2015
Closed
@mnapoli mnapoli mentioned this pull request May 21, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c: Design / UI For issues that impact Matomo's user interface or the design overall. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review
Projects
None yet
Milestone
2.14.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mnapoli @diosmosis @tsteur @mattab