Hi Surinder,
Thanks for your review.
We have changed the indentation for _dashboard.scss file and also
removed the style about icon-postgres:before, like margin-top,etc, but we
are not sure if it is perfectly aligned now, you can add further change to
it.
As the second comment, I'm sorry I'm not sure what's the problem, could
you please clarify it? Because we replace css with scss right now,
dashboard.css doesn't exist. So maybe you are looking at the css file that
are complied by the scss?
For the fourth comment, we tried the steps you suggested on master
branch, the error is not shown either. So it should be an existing issue.
But if you want to see the error message, navigate to "Servers" at the top
of browser, then navigate back to postgresql server, then you will see the
error message.
Thanks,
Wenlin &Violet
On Mon, Aug 7, 2017 at 2:30 PM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:
> Hi
> Review comments:
>
> 1.
>
> For consistency, we use two spaces for indentation in CSS files. Four
> spaces are used in _dashboard.scss file. The configurations are
> defined in web/.editorconfig file.
> 2.
>
> In,dashboard.css Can we return function object in return instead of
> function class itself, this will eliminate the need of creating function
> object every time we use info and error?
> 3.
>
> On Dashboard, I can see Postgres icon is misaligned compared to other
> icons in Getting Started section. It is not related to this patch.
> adjusting margin top will fix it.
> 4.
>
> I tried to test out Error message displayed, but I encounter an
> error(screenshot attached).
> Steps to reproduce:
> - Open pgAdmin4 in browser
> - Connect to PostgreSQL Server, Keep dashboard tab open.
> - Navigate to the database which is connected.
> - Now disconnect pgAdmin4 python server.
> - No error message is displayed on Dashboard because it breaks in
> JS as xhr.responseText is empty.
> However, it might be an existing issue.
>
> Thanks,
> Surinder
>
> On Mon, Aug 7, 2017 at 10:40 AM, Wenlin Zhang <wzh...@pivotal.io> wrote:
>
> Hi Ashesh,
>>
>> That's correct. This patch just changed alert style in the 'tabs',
>> such as Dependency and Dependents.
>>
>> Thanks
>>
>> Wenlin
>>
>> On Mon, Aug 7, 2017 at 12:51 PM, Ashesh Vashi <
>> ashesh.va...@enterprisedb.com> wrote:
>>
>>> Surinder,
>>>
>>> Please take a look at this patch.
>>>
>>> If I recalls correctly, this patch is related to styling of the 'tabs'
>>> shown on the main window.
>>> Wenlin - please correct me if my understanding is wrong.
>>>
>>> --
>>>
>>> Thanks & Regards,
>>>
>>> Ashesh Vashi
>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>> <http://www.enterprisedb.com>
>>>
>>>
>>> *http://www.linkedin.com/in/asheshvashi*
>>> <http://www.linkedin.com/in/asheshvashi>
>>>
>>> On Mon, Aug 7, 2017 at 8:11 AM, Sarah McAlear <smcal...@pivotal.io>
>>> wrote:
>>>
>>>> Hi hackers,
>>>>
>>>> Could you please review this patch?
>>>>
>>>> Thanks
>>>>
>>>> Wenlin and Sarah
>>>>
>>>> On Wed, Aug 2, 2017 at 2:15 PM, Wenlin Zhang <wzh...@pivotal.io> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> This patch changes the alert style in the sub-navigation to match
>>>>> style guide.
>>>>>
>>>>> Thanks,
>>>>> Wenlin, Shirley & Sarah
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
diff --git a/web/pgadmin/browser/static/js/browser.js
b/web/pgadmin/browser/static/js/browser.js
index 45939b00..23d1d50e 100644
--- a/web/pgadmin/browser/static/js/browser.js
+++ b/web/pgadmin/browser/static/js/browser.js
@@ -136,7 +136,7 @@ define(
width: 500,
isCloseable: false,
isPrivate: true,
- content: '<div><div class="alert alert-info pg-panel-message
pg-panel-statistics-message">' + select_object_msg + '</div><div
class="pg-panel-statistics-container hidden"></div></div>',
+ content: '<div><div class="alert alert-info pg-panel-message
pg-panel-statistics-message font-primary-blue border-blue-2 text-14">' +
select_object_msg + '</div><div class="pg-panel-statistics-container
hidden"></div></div>',
events: panelEvents
}),
// Reversed engineered SQL for the object
@@ -157,7 +157,7 @@ define(
width: 500,
isCloseable: false,
isPrivate: true,
- content: '<div><div class="alert alert-info pg-panel-message
pg-panel-depends-message">' + select_object_msg + '</div><div
class="pg-panel-depends-container hidden"></div></div>',
+ content: '<div><div class="alert alert-info pg-panel-message
pg-panel-depends-message font-primary-blue border-blue-2 text-14">' +
select_object_msg + '</div><div class="pg-panel-depends-container
hidden"></div></div>',
events: panelEvents
}),
// Dependents of the object
@@ -168,7 +168,7 @@ define(
width: 500,
isCloseable: false,
isPrivate: true,
- content: '<div><div class="alert alert-info pg-panel-message
pg-panel-depends-message">' + select_object_msg + '</div><div
class="pg-panel-depends-container hidden"></div></div>',
+ content: '<div><div class="alert alert-info pg-panel-message
pg-panel-depends-message font-primary-blue border-blue-2 text-14">' +
select_object_msg + '</div><div class="pg-panel-depends-container
hidden"></div></div>',
events: panelEvents
})
},
diff --git a/web/pgadmin/dashboard/__init__.py
b/web/pgadmin/dashboard/__init__.py
index 8ebab16c..24cc86ef 100644
--- a/web/pgadmin/dashboard/__init__.py
+++ b/web/pgadmin/dashboard/__init__.py
@@ -45,10 +45,7 @@ class DashboardModule(PgAdminModule):
Returns:
list: the stylesheets used by this module.
"""
- stylesheets = [
- url_for('dashboard.static', filename='css/dashboard.css')
- ]
- return stylesheets
+ return []
def get_panels(self):
return [
diff --git a/web/pgadmin/dashboard/static/css/dashboard.css
b/web/pgadmin/dashboard/static/css/dashboard.css
deleted file mode 100644
index 735aebf5..00000000
--- a/web/pgadmin/dashboard/static/css/dashboard.css
+++ /dev/null
@@ -1,90 +0,0 @@
-.dashboard-container {
- padding: 15px;
-}
-
-.dashboard-header-spacer {
- padding-top: 15px;
-}
-
-.dashboard-link {
- text-align: center;
-}
-
-.dashboard-link a {
- cursor: pointer;
-}
-
-.dashboard-icon {
- color: black;
-}
-
-.dashboard-tab-container {
- border-left: 1px solid #E2E2E2;
- border-right: 1px solid #E2E2E2;
- border-bottom: 1px solid #E2E2E2;
-}
-
-.dashboard-tab-panel > li > a {
- padding: 0px 15px !important;
-}
-
-.dashboard-tab {
- border: 0px;
- border-radius: 4px 4px 0px 0px;
- margin-right: 1px;
- font-size: 13px;
- line-height: 25px;
- margin-top: 5px;
-}
-
-.dashboard-tab.active {
- margin-top: 0px;
- line-height: 30px;
-}
-
-.dashboard-tab-btn-group {
- background-color: #D2D2D2;
- border: 2px solid #A9A9A9;
- left: 0px;
- right: 0px;
- padding: 2px;
-}
-
-.dashboard-tab-btn-group button {
- padding: 5px;
-}
-
-.dashboard-tab-btn-group > button {
- margin: 2px 3px 2px 0px;
- min-width: 40px;
-}
-
-.dashboard-tab-btn-group > button:first-child {
- margin-left: 3px;
-}
-
-.dashboard-tab-btn-group > button:last-child {
- margin-right: 3px;
-}
-
-.graph-container {
- margin-top: 10px;
- height: 150px;
-}
-
-.graph-error {
- background-color: #E2E2E2;
- padding-top: 20px
-}
-
-.grid-error {
- background-color: #E2E2E2;
- padding-top: 20px;
- padding-bottom: 40px;
-}
-
-.icon-postgres:before {
- height: 43px;
- margin-top: 13px;
- display: block;
-}
diff --git a/web/pgadmin/dashboard/static/js/dashboard.js
b/web/pgadmin/dashboard/static/js/dashboard.js
index 86ebe38a..20dc5903 100644
--- a/web/pgadmin/dashboard/static/js/dashboard.js
+++ b/web/pgadmin/dashboard/static/js/dashboard.js
@@ -1,11 +1,11 @@
define('pgadmin.dashboard', [
'sources/url_for', 'sources/gettext', 'require', 'jquery', 'underscore',
- 'pgadmin', 'backbone', 'backgrid', 'flotr2',
+ 'pgadmin', 'backbone', 'backgrid', 'sources/alerts/dashboard', 'flotr2',
'pgadmin.alertifyjs', 'backgrid.filter',
'pgadmin.browser', 'bootstrap', 'wcdocker'
],
-function(url_for, gettext, r, $, _, pgAdmin, Backbone, Backgrid, Flotr,
- alertify) {
+function(url_for, gettext, r, $, _, pgAdmin, Backbone, Backgrid,
AlertDashboard,
+ Flotr, alertify) {
var wcDocker = window.wcDocker,
pgBrowser = pgAdmin.Browser;
@@ -327,21 +327,21 @@ function(url_for, gettext, r, $, _, pgAdmin, Backbone,
Backgrid, Flotr,
var err = $.parseJSON(xhr.responseText),
msg = err.errormsg,
cls;
+ var html = "";
+ var alertDashboard = new AlertDashboard();
// If we get a 428, it means the server isn't connected
if (xhr.status == 428) {
if (_.isUndefined(msg) || _.isNull(msg)) {
msg = gettext('Please connect to the selected
server to view the graph.');
}
- cls = 'info';
- } else {
+ html = alertDashboard.info(msg);
+ } else {
msg = gettext('An error occurred whilst rendering
the graph.');
- cls = 'danger';
+ html = alertDashboard.error(msg);
}
$(container).addClass('graph-error');
- $(container).html(
- '<div class="alert alert-' + cls + '
pg-panel-message" role="alert">' + msg + '</div>'
- );
+ $(container).html(html);
// Try again...
if (container.clientHeight > 0 &&
container.clientWidth > 0) {
@@ -447,18 +447,21 @@ function(url_for, gettext, r, $, _, pgAdmin, Backbone,
Backgrid, Flotr,
filter.search();
},
error: function(model, xhr, options) {
- var err = $.parseJSON(xhr.responseText),
+ var err = $.parseJSON(xhr.responseText),
msg = err.errormsg,
- cls;
+ cls;
+
+ var html = "";
+ var alertDashboard = new AlertDashboard();
// If we get a 428, it means the server isn't connected
if (xhr.status == 428) {
if (_.isUndefined(msg) || _.isNull(msg)) {
msg = gettext('Please connect to the selected
server to view the table.');
}
- cls = 'info';
+ html = alertDashboard.info(msg);
} else {
msg = gettext('An error occurred whilst rendering the
table.');
- cls = 'danger';
+ html = alertDashboard.error(msg);
}
// Replace the content with the error, if not already
present. Always update the message
@@ -467,9 +470,7 @@ function(url_for, gettext, r, $, _, pgAdmin, Backbone,
Backgrid, Flotr,
$(container).addClass('grid-error');
}
- $(container).html(
- '<div class="alert alert-' + cls + ' pg-panel-message"
role="alert">' + msg + '</div>'
- );
+ $(container).html(html);
// Try again
setTimeout(function() {
diff --git a/web/pgadmin/static/css/style.css b/web/pgadmin/static/css/style.css
index 7d651925..6dd7e560 100644
--- a/web/pgadmin/static/css/style.css
+++ b/web/pgadmin/static/css/style.css
@@ -25,7 +25,6 @@
@import '../../preferences/static/css/preferences.css';
@import '../../browser/static/css/browser.css';
-@import '../../dashboard/static/css/dashboard.css';
@import '../../browser/static/css/wizard.css';
@import '../../tools/debugger/static/css/debugger.css';
@@ -34,4 +33,4 @@
@import '../../tools/sqleditor/static/css/sqleditor.css';
@import '../../misc/static/explain/css/explain.css';
@import '../../misc/bgprocess/static/css/bgprocess.css';
-@import '../../misc/static/explain/css/explain.css';
\ No newline at end of file
+@import '../../misc/static/explain/css/explain.css';
diff --git a/web/pgadmin/static/js/alerts/dashboard.js
b/web/pgadmin/static/js/alerts/dashboard.js
new file mode 100644
index 00000000..ddb0c231
--- /dev/null
+++ b/web/pgadmin/static/js/alerts/dashboard.js
@@ -0,0 +1,45 @@
+define([
+ 'jquery',
+], function ($) {
+ var DashboardAlert = function () {
+ var info = function (message) {
+ var html = '<div class="alert-row" role="alert">' +
+ '<div class="alert alert-info border-blue-2 font-primary-blue
text-14 alert-box">' +
+ '<div class="media">' +
+ '<div class="media-body media-middle">' +
+ '<div class="alert-text">' +
+ message +
+ '</div>' +
+ '</div>' +
+ '</div>' +
+ '</div>' +
+ '</div>';
+ return html;
+ };
+
+ var error = function(message) {
+ var html = '<div class="alert-row" role="alert">' +
+ '<div class="alert alert-danger border-red-2 font-red-3 text-14
alert-box">' +
+ '<div class="media">' +
+ '<div class="media-body media-middle">' +
+ '<div class="alert-icon error-icon">' +
+ '<i class="fa fa-exclamation-triangle"
aria-hidden="true"></i>' +
+ '</div>' +
+ '<div class="alert-text">' +
+ message +
+ '</div>' +
+ '</div>' +
+ '</div>' +
+ '</div>' +
+ '</div>';
+ return html;
+ };
+
+ $.extend(this, {
+ 'info': info,
+ 'error': error,
+ });
+ };
+
+ return DashboardAlert;
+});
\ No newline at end of file
diff --git a/web/pgadmin/static/scss/_alert.scss
b/web/pgadmin/static/scss/_alert.scss
index fdd4546c..212003fa 100644
--- a/web/pgadmin/static/scss/_alert.scss
+++ b/web/pgadmin/static/scss/_alert.scss
@@ -51,42 +51,38 @@ category: alerts
```
*/
-
// from bootstrap scss:
-@if $enable-flex {
- .media {
- display: flex;
- }
- .media-body {
- flex: 1;
- }
- .media-middle {
- align-self: center;
- }
- .media-bottom {
- align-self: flex-end;
- }
-} @else {
- .media,
- .media-body {
- overflow: hidden;
- }
- .media-body {
- width: 10000px;
- }
- .media-left,
- .media-right,
- .media-body {
- display: inline;
- vertical-align: top;
- }
- .media-middle {
- vertical-align: middle;
- }
- .media-bottom {
- vertical-align: bottom;
- }
+.media-body {
+ vertical-align: top;
+ width: initial;
+}
+
+.media-middle {
+ align-self: center;
+}
+
+.media-bottom {
+ align-self: flex-end;
+}
+
+.media-body.media-middle {
+ display: flex;
+ align-items: center;
+ width: 100%;
+}
+
+.alert-icon {
+ display: flex;
+ align-items: center;
+ color: white;
+ padding: 15px 15px 15px 17px;
+ width: 50px;
+ min-height: 50px;
+ font-size: 14px;
+ text-align: center;
+ align-self: stretch;
+ flex-shrink: 0;
}
.alert-row {
@@ -103,22 +99,12 @@ category: alerts
padding: 15px;
}
-.alert-icon {
- display: inline-block;
- color: white;
- padding: 15px;
- width: 50px;
- height: 50px;
- font-size: 14px;
- text-align: center;
-}
-
.success-icon {
- background: #3a773a;
+ background: $color-green-3;
}
.error-icon {
- background: #d0021b;
+ background: $color-red-3;
}
.info-icon {
@@ -133,12 +119,21 @@ category: alerts
}
.alert-info {
- border-color: #84acdd
+ border-color: $color-blue-2;
+ background-image: none;
}
-.media-body {
- vertical-align: top;
- width: initial;
+.alert-danger {
+ background-image: none;
+}
+
+.grid-error, .graph-error {
+ .alert-row {
+ align-items: center;
+ height: 100%;
+ display: flex;
+ justify-content: center;
+ }
}
.ajs-message {
@@ -155,11 +150,12 @@ category: alerts
}
.alert-icon {
- padding: 8px;
+ padding: 8px 8px 8px 10.5px;
width: 35px;
height: 35px;
border-top-left-radius: 4px;
border-bottom-left-radius: 4px;
+ min-height: auto;
}
.alert-text {
@@ -167,8 +163,8 @@ category: alerts
border: 1px solid $color-red-2;
border-top-right-radius: 4px;
border-bottom-right-radius: 4px;
- padding: 7px 12px 5px 10px;
- border-left: 0px;
+ padding: 7px 12px 6px 10px;
+ border-left: none;
}
.error-in-footer {
@@ -193,7 +189,17 @@ category: alerts
height: 35px;
.alert-text {
- border: 0px;
+ border: none;
}
}
}
+
+//Internet Explorer specific CSS
+@media screen and (-ms-high-contrast: active), (-ms-high-contrast: none) {
+ .alert-danger {
+ width: 90%;
+ }
+ .alert-info {
+ width: 90%;
+ }
+}
diff --git a/web/pgadmin/static/scss/_othercolors.scss
b/web/pgadmin/static/scss/_othercolors.scss
index 9f6fbcda..b284346b 100644
--- a/web/pgadmin/static/scss/_othercolors.scss
+++ b/web/pgadmin/static/scss/_othercolors.scss
@@ -79,10 +79,12 @@ These colors should be used to highlight hover options in
dropdown menus and cat
width: 100%;
}
+$color-blue-2: #84acdd;
$color-red-1: #f2dede;
$color-red-2: #de8585;
$color-red-3: #d0021b;
$color-green-2: #a2c189;
+$color-green-3: #3a773a;
.bg-white-1 {
background-color: #ffffff;
@@ -93,7 +95,7 @@ $color-green-2: #a2c189;
}
.bg-blue-2 {
- background-color: #84acdd;
+ background-color: $color-blue-2;
}
.bg-red-1 {
@@ -117,7 +119,7 @@ $color-green-2: #a2c189;
}
.bg-green-3 {
- background-color: #3a773a;
+ background-color: $color-green-3;
}
.border-blue-1 {
@@ -125,7 +127,7 @@ $color-green-2: #a2c189;
}
.border-blue-2 {
- border-color: #84acdd;
+ border-color: $color-blue-2;
}
.border-red-1 {
@@ -149,7 +151,7 @@ $color-green-2: #a2c189;
}
.border-green-3 {
- border-color: #3a773a;
+ border-color: $color-green-3;
}
.font-red-3 {
@@ -157,7 +159,7 @@ $color-green-2: #a2c189;
}
.font-green-3 {
- color: #3a773a;
+ color: $color-green-3;
}
.font-white {
diff --git a/web/pgadmin/static/scss/dashboard/_dashboard.scss
b/web/pgadmin/static/scss/dashboard/_dashboard.scss
new file mode 100644
index 00000000..16c30939
--- /dev/null
+++ b/web/pgadmin/static/scss/dashboard/_dashboard.scss
@@ -0,0 +1,84 @@
+.dashboard-container {
+ padding: 15px;
+}
+
+.dashboard-header-spacer {
+ padding-top: 15px;
+}
+
+.dashboard-link {
+ text-align: center;
+}
+
+.dashboard-link a {
+ cursor: pointer;
+}
+
+.dashboard-icon {
+ color: black;
+}
+
+.dashboard-tab-container {
+ border-left: 1px solid $color-gray-2;
+ border-right: 1px solid $color-gray-2;
+ border-bottom: 1px solid $color-gray-2;
+}
+
+.dashboard-tab-panel > li > a {
+ padding: 0px 15px !important;
+}
+
+.dashboard-tab {
+ border: 0px;
+ border-radius: 4px 4px 0px 0px;
+ margin-right: 1px;
+ font-size: 13px;
+ line-height: 25px;
+ margin-top: 5px;
+}
+
+.dashboard-tab.active {
+ margin-top: 0px;
+ line-height: 30px;
+}
+
+.dashboard-tab-btn-group {
+ background-color: $color-gray-3;
+ border: 1px solid $color-gray-4;
+ left: 0px;
+ right: 0px;
+ padding: 2px;
+}
+
+.dashboard-tab-btn-group button {
+ padding: 5px;
+}
+
+.dashboard-tab-btn-group > button {
+ margin: 2px 3px 2px 0px;
+ min-width: 40px;
+}
+
+.dashboard-tab-btn-group > button:first-child {
+ margin-left: 3px;
+}
+
+.dashboard-tab-btn-group > button:last-child {
+ margin-right: 3px;
+}
+
+.graph-container {
+ margin-top: 10px;
+ height: 150px;
+}
+
+.graph-error {
+ background-color: $color-gray-2;
+ padding-top: 20px
+}
+
+.grid-error {
+ background-color: $color-gray-2;
+ padding-top: 47px;
+ padding-bottom: 31px;
+}
diff --git a/web/pgadmin/static/scss/pgadmin.scss
b/web/pgadmin/static/scss/pgadmin.scss
index 42f04d2a..13968a2b 100644
--- a/web/pgadmin/static/scss/pgadmin.scss
+++ b/web/pgadmin/static/scss/pgadmin.scss
@@ -18,3 +18,5 @@ $enable-flex: true;
@import 'alertify.overrides';
@import 'backform.overrides';
@import 'sqleditor/history';
+
+@import 'dashboard/dashboard';
diff --git a/web/regression/javascript/alerts/alertify_wrapper_spec.js
b/web/regression/javascript/alerts/alertify_wrapper_spec.js
deleted file mode 100644
index 0701b1cc..00000000
--- a/web/regression/javascript/alerts/alertify_wrapper_spec.js
+++ /dev/null
@@ -1,37 +0,0 @@
-//////////////////////////////////////////////////////////////////////////
-//
-// pgAdmin 4 - PostgreSQL Tools
-//
-// Copyright (C) 2013 - 2017, The pgAdmin Development Team
-// This software is released under the PostgreSQL Licence
-//
-//////////////////////////////////////////////////////////////////////////
-import alertify from 'pgadmin.alertifyjs';
-
-describe('alertify_wrapper', function () {
- describe('success', function () {
- it('calls the success function from alertify and adds the checkmark to the
element', function () {
- spyOn(alertify, 'orig_success');
-
- alertify.success('Yay, congrats!', 1);
-
- var calledWithMessage = alertify.orig_success.calls.mostRecent().args[0];
-
- expect(calledWithMessage).toContain('Yay, congrats!');
- expect(calledWithMessage).toContain('class="fa fa-check"');
- });
- });
-
- describe('error', function () {
- it('calls the error function from alertify and adds the warning symbol to
the element', function () {
- spyOn(alertify, 'orig_error');
-
- alertify.error('bad, very bad', 1);
-
- var calledWithMessage = alertify.orig_error.calls.mostRecent().args[0];
-
- expect(calledWithMessage).toContain('bad, very bad');
- expect(calledWithMessage).toContain('class="fa
fa-exclamation-triangle"');
- });
- });
-});
