Re: [pgAdmin4][Patch]: RM-2859: Can't create new user

On Tue, Nov 28, 2017 at 3:40 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Tue, Nov 28, 2017 at 7:10 AM, Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> On Mon, Nov 27, 2017 at 5:01 PM, Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Mon, Nov 27, 2017 at 4:58 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Nov 27, 2017 at 11:26 AM, Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Mon, Nov 27, 2017 at 4:47 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Nov 27, 2017 at 11:03 AM, Khushboo Vashi <
>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Nov 27, 2017 at 4:13 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, Nov 27, 2017 at 10:39 AM, Khushboo Vashi <
>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, Nov 27, 2017 at 2:59 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Mon, Nov 27, 2017 at 9:19 AM, Khushboo Vashi <
>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Nov 27, 2017 at 2:20 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, Nov 27, 2017 at 5:25 AM, Khushboo Vashi <
>>>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Fri, Nov 24, 2017 at 3:21 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Thu, Nov 23, 2017 at 10:43 AM, Khushboo Vashi <
>>>>>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Thu, Nov 23, 2017 at 2:58 PM, Dave Page <
>>>>>>>>>>>>>>> dpage(at)pgadmin(dot)org> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Thu, Nov 23, 2017 at 5:03 AM, Khushboo Vashi <
>>>>>>>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)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
>