Re: [pgAdmin4][patch] update the alert style in the sub-navigation

Hi Surinder!

Are you referring to the green message popup? If so, it also appears to be
happening on master. We'll log a bug in our backlog and Redmine and
prioritize it. We agree that it needs to be fixed, but don't think it's
unrelated to this patch.

Thanks!
Violet & Sarah

On Thu, Aug 10, 2017 at 2:32 PM, Surinder Kumar <
surinder(dot)kumar(at)enterprisedb(dot)com> wrote:

> Please find attached screenshot.
>
> On Thu, Aug 10, 2017 at 11:30 AM, Surinder Kumar <
> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>
>> Hi Sarah,
>>
>> We noticed that due to this patch, the alert style of "Database
>> connected" message is changed.
>> Can you please look into this?
>>
>> Thanks,
>> Surinder
>>
>> On Wed, Aug 9, 2017 at 4:43 PM, Surinder Kumar <
>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>>> ​Hi,
>>>
>>> ​The updated patch looks good to me.
>>>
>>> On Wed, Aug 9, 2017 at 4:15 PM, Sarah McAlear <smcalear(at)pivotal(dot)io>
>>> wrote:
>>>
>>>> As discussed with Surinder, we have created a Redmine ticket for his
>>>> 4th comment regarding the error message not showing up when the app can't
>>>> be reached. This issue existed prior to this patch and should be
>>>> prioritized.
>>>>
>>>> https://redmine.postgresql.org/issues/2640
>>>>
>>>> Thanks!
>>>> Matt & Sarah
>>>>
>>>> On Wed, Aug 9, 2017 at 4:06 PM, Sarah McAlear <smcalear(at)pivotal(dot)io>
>>>> wrote:
>>>>
>>>>> Hi Surinder!
>>>>>
>>>>> I am not able to see anything different from what I see on Master with
>>>>> or without the patch applied. I tried adjusting the preferences. I did
>>>>> update the dashboard.js to instantiate a new object, great idea!
>>>>>
>>>>> Thanks,
>>>>> Sarah
>>>>>
>>>>>
>>>>>
>>>>> On Wed, Aug 9, 2017 at 1:42 PM, Surinder Kumar <
>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Wenlin,
>>>>>>
>>>>>> On Tue, Aug 8, 2017 at 3:15 PM, Wenlin Zhang <wzhang(at)pivotal(dot)io>
>>>>>> wrote:
>>>>>>
>>>>>>> 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?
>>>>>>>
>>>>>> ​Sorry I​ typed 'dashboard.css' instead of 'dashboard.js'.
>>>>>> ​In dashboard.js can we change `return DashboardAlert;` to `return
>>>>>> new DashboardAlert();`
>>>>>> and then we can remove the instances being created(var alertDashboard
>>>>>> = new AlertDashboard();) from dashboard.js, and simply
>>>>>> use `AlertDashboard.info('message')`.
>>>>>>
>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>> ​The error in console will appear when you selected a database which
>>>>>> is connected and stop the backend Python server because the request for
>>>>>> graphs for database level will fail and there is no response returned from
>>>>>> server.
>>>>>> It might be not reproducible to you If you set the refresh rate to
>>>>>> higher value (i.e. Preferences > Graphs) in preferences. Just set refresh
>>>>>> rate to 1 for all dashboard graphs and repeat the steps.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Wenlin &Violet
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Aug 7, 2017 at 2:30 PM, Surinder Kumar <
>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)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.
>>>>>>>> ​
>>>>>>>>
>>>>>>>> ​Here I mean Stop Python server. I​
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> 1.
>>>>>>>> - ​
>>>>>>>> - 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 <wzhang(at)pivotal(dot)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(dot)vashi(at)enterprisedb(dot)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 <
>>>>>>>>>> smcalear(at)pivotal(dot)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 <wzhang(at)pivotal(dot)io>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>
>>>>>>>>>>>> This patch changes the alert style in the sub-navigation to
>>>>>>>>>>>> match style guide.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Wenlin, Shirley & Sarah
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> ​
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>