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

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(dot)vashi(at)enterprisedb(dot)com> wrote:

>
>
> 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
>>
>
>