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

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