Hi Dave, Please find the attached updated patch, which includes:
- The fix for this RM - Close button for the error message, which is applicable globally Thanks, Khushboo On Tue, Nov 28, 2017 at 4:18 PM, Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote: > > > On Tue, Nov 28, 2017 at 3:40 PM, Dave Page <dp...@pgadmin.org> wrote: > >> >> >> On Tue, Nov 28, 2017 at 7:10 AM, Khushboo Vashi < >> khushboo.va...@enterprisedb.com> wrote: >> >>> >>> >>> On Mon, Nov 27, 2017 at 5:01 PM, Khushboo Vashi < >>> khushboo.va...@enterprisedb.com> wrote: >>> >>>> >>>> >>>> On Mon, Nov 27, 2017 at 4:58 PM, Dave Page <dp...@pgadmin.org> wrote: >>>> >>>>> >>>>> >>>>> On Mon, Nov 27, 2017 at 11:26 AM, Khushboo Vashi < >>>>> khushboo.va...@enterprisedb.com> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Mon, Nov 27, 2017 at 4:47 PM, Dave Page <dp...@pgadmin.org> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Mon, Nov 27, 2017 at 11:03 AM, Khushboo Vashi < >>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Mon, Nov 27, 2017 at 4:13 PM, Dave Page <dp...@pgadmin.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Nov 27, 2017 at 10:39 AM, Khushboo Vashi < >>>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Mon, Nov 27, 2017 at 2:59 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Mon, Nov 27, 2017 at 9:19 AM, Khushboo Vashi < >>>>>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Mon, Nov 27, 2017 at 2:20 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Mon, Nov 27, 2017 at 5:25 AM, Khushboo Vashi < >>>>>>>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi Dave, >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Fri, Nov 24, 2017 at 3:21 PM, Dave Page <dp...@pgadmin.org >>>>>>>>>>>>>> > wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Thu, Nov 23, 2017 at 10:43 AM, Khushboo Vashi < >>>>>>>>>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Thu, Nov 23, 2017 at 2:58 PM, Dave Page < >>>>>>>>>>>>>>>> dp...@pgadmin.org> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hi >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Thu, Nov 23, 2017 at 5:03 AM, Khushboo Vashi < >>>>>>>>>>>>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Please find the attached patch to fix RM #2859: Can't >>>>>>>>>>>>>>>>>> create new user. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> The "User Management" dialogue footer was overlapping the >>>>>>>>>>>>>>>>>> back-grid table which has been fixed. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> If my screen is too small, it now looks like the attached >>>>>>>>>>>>>>>>> screenshot, which is really quite ugly. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> If we don't leave the bottom blank space then in case of >>>>>>>>>>>>>>>> error the error-message will shown on the grid itself and user >>>>>>>>>>>>>>>> can't >>>>>>>>>>>>>>>> perform any task. >>>>>>>>>>>>>>>> Please refer the attached screen-shot for the same. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Right, but we also can't have that space left blank like >>>>>>>>>>>>>>> that. Can't we extend the scroll range of the grid? In other >>>>>>>>>>>>>>> words, always >>>>>>>>>>>>>>> include space for an extra row or so, so it can scroll above >>>>>>>>>>>>>>> the error >>>>>>>>>>>>>>> message, when, and only when a message is shown? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Please find the attached screen-shot, If we always include an >>>>>>>>>>>>>> extra row. >>>>>>>>>>>>>> Suggestion please. >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> I think that's much better, though still not ideal. What if we >>>>>>>>>>>>> made the error messages closable like other notifications? >>>>>>>>>>>>> >>>>>>>>>>>>> The error-messages in pgAdmin 4 are not closable, so it will >>>>>>>>>>>> not go with the flow. >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> I meant to do it globally. >>>>>>>>>>> >>>>>>>>>>> Should I create the separate case for this? >>>>>>>>>> >>>>>>>>> >>>>>>>>> No, I don't think there's any need for that. >>>>>>>>> >>>>>>>>> Does it seem like it would solve the problem appropriately? >>>>>>>>> >>>>>>>>> >>>>>>>> It would lead us to more complexity >>>>>>>> 1. How can we keep track of the closed error messages for multiple >>>>>>>> fields? >>>>>>>> >>>>>>> >>>>>>> Do we need to? >>>>>>> >>>>>>> >>>>>>>> 2. We have validated backbone model on focus out/change, so we need >>>>>>>> to change the basic error model. >>>>>>>> >>>>>>> >>>>>>> I'm not sure why. Can't we just have an X button on the error panel >>>>>>> that will hide it? If another error occurs (e.g. because the user >>>>>>> changes >>>>>>> focus), just re-display it. >>>>>>> >>>>>>> >>>>>> This means, if the error message is displayed for the field 1 and >>>>>> after closing if we go ahead without filling up the valid data, on the >>>>>> focus out; the same error message will be shown. >>>>>> >>>>> >>>>> Yes. >>>>> >>>>> >>>>>> >>>>>> I was thinking; if we have closed the error message for the field 1, >>>>>> then it will not display any kind of message for that particular field. >>>>>> So, >>>>>> I have mentioned about the complexity. >>>>>> >>>>> >>>>> I'm not convinced we need that level of complexity. >>>>> >>>>> Can you whip up a PoC so we can see how it behaves? >>>>> >>>>> Please find the attached patch. >>> >>> This patch includes: >>> - The fix for this RM >>> - Close button for the error message for the User management module >>> - Close button for other node modules like server , schema etc.... >>> >> >> I like that - it seems to solve the problem quite elegantly. >> >> Are you happy with the patch or is there other work you'd like to do >> before commit? >> >> I would like to check all the dialogues other than nodes, if required I > will apply this change to them. > >> Thanks! >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > >
diff --git a/web/pgadmin/browser/static/js/node.js b/web/pgadmin/browser/static/js/node.js index a9c2b18..7b5ba6f 100644 --- a/web/pgadmin/browser/static/js/node.js +++ b/web/pgadmin/browser/static/js/node.js @@ -265,10 +265,16 @@ define( <i class="fa fa-exclamation-triangle" aria-hidden="true"></i>\ </div>\ <div class="alert-text">' + msg + '</div>\ + <div class="close-error-bar">\ + <a class="close-error">x</a>\ + </div>\ </div>\ </div>'; if(!_.isUndefined(that.statusBar)) { that.statusBar.html(alertMessage).css("visibility", "visible"); + that.statusBar.find("a.close-error").bind("click", function(e) { + this.empty().css("visibility", "hidden"); + }.bind(that.statusBar)); } callback(true); diff --git a/web/pgadmin/browser/static/js/wizard.js b/web/pgadmin/browser/static/js/wizard.js index a65c71c..d088aa7 100644 --- a/web/pgadmin/browser/static/js/wizard.js +++ b/web/pgadmin/browser/static/js/wizard.js @@ -96,6 +96,9 @@ function(_, Backbone, pgAdmin, pgBrowser) { + " </div>" + " <div class='alert-text'>" + " </div>" + + " <div class='close-error-bar'>" + + " <a class='close-error'>x</a>" + + " </div>" + " </div>" + " </div>" + " </div>" @@ -128,6 +131,7 @@ function(_, Backbone, pgAdmin, pgBrowser) { "click button.wizard-maximize-event" : "onMaximize", "click button.wizard-finish" : "finishWizard", "click button.wizard-help" : "onDialogHelp", + "click a.close-error" : "closeErrorMsg", }, initialize: function(options) { this.options = _.extend({}, this.options, options.options); @@ -224,6 +228,10 @@ function(_, Backbone, pgAdmin, pgBrowser) { this.options.disable_prev = false; } }, + closeErrorMsg: function() { + $(this.el).find('.pg-prop-status-bar .alert-text').empty(); + $(this.el).find('.pg-prop-status-bar').css("visibility", "hidden"); + }, beforeNext: function(){ return this.evalASFunc(this.currPage.beforeNext); }, diff --git a/web/pgadmin/static/css/bootstrap.overrides.css b/web/pgadmin/static/css/bootstrap.overrides.css index b894021..b0c531a 100755 --- a/web/pgadmin/static/css/bootstrap.overrides.css +++ b/web/pgadmin/static/css/bootstrap.overrides.css @@ -1176,6 +1176,16 @@ form[name="change_password_form"] .help-block { overflow: hidden; } +.close-error-bar { + background: #d0021b; + padding: 5px; +} + +.close-error-bar a { + color: #FFFFFF; + cursor: pointer; +} + .user_management .search_users form { margin: 0; } diff --git a/web/pgadmin/static/scss/_alert.scss b/web/pgadmin/static/scss/_alert.scss index 3f860b8..441c40f 100644 --- a/web/pgadmin/static/scss/_alert.scss +++ b/web/pgadmin/static/scss/_alert.scss @@ -104,8 +104,6 @@ .alert-text { flex-grow: 1; border: 1px solid $color-red-2; - border-top-right-radius: 4px; - border-bottom-right-radius: 4px; padding: 7px 12px 6px 10px; border-left: none; } diff --git a/web/pgadmin/tools/user_management/static/js/user_management.js b/web/pgadmin/tools/user_management/static/js/user_management.js index ffdf8b2..50d1465 100644 --- a/web/pgadmin/tools/user_management/static/js/user_management.js +++ b/web/pgadmin/tools/user_management/static/js/user_management.js @@ -392,8 +392,8 @@ define([ prepare: function() { var self = this, footerTpl = _.template([ - '<div class="pg-prop-footer">', - '<div class="pg-prop-status-bar" style="visibility:hidden">', + '<div class="pg-prop-footer" style="visibility:hidden;">', + '<div class="pg-prop-status-bar">', '<div class="media error-in-footer bg-red-1 border-red-2 font-red-3 text-14">', '<div class="media-body media-middle">', '<div class="alert-icon error-icon">', @@ -401,12 +401,12 @@ define([ '</div>', '<div class="alert-text">', '</div>', + '<div class="close-error-bar"><a class="close-error">x</a></div>', '</div>', '</div>', '</div>', '</div>'].join("\n")), - $footer = $(footerTpl()), - $statusBar = $footer.find('.pg-prop-status-bar'), + $statusBar = $(footerTpl()), UserRow = Backgrid.Row.extend({ userInvalidColor: "lightYellow", @@ -558,7 +558,7 @@ define([ this.$content = $("<div class='user_management object subnode'></div>").append( headerTpl(data)).append($gridBody - ).append($footer); + ).append($statusBar); $(this.elements.body.childNodes[0]).addClass( 'alertify_tools_dialog_backgrid_properties'); @@ -571,6 +571,11 @@ define([ userCollection.fetch(); + this.$content.find('a.close-error').click(function(e) { + $statusBar.find('.alert-text').empty(); + $statusBar.css("visibility", "hidden"); + }); + this.$content.find('button.add').first().click(function(e) { e.preventDefault(); var canAddRow = true;