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

Hi!

We fixed that issue and created a new patch
<https://www.postgresql.org/message-id/flat/CAGRPzo9wUDtYE2jFz-%3DPbJr%2Btf20zyFncdEoAFdVMkTgoXzf3w%40mail.gmail.com>
.

Thanks!

On Thu, Aug 10, 2017 at 4:10 PM, Violet Cheng <vcheng(at)pivotal(dot)io> wrote:

> Here's the Redmine link
>
> https://redmine.postgresql.org/issues/2644
>
> On Thu, Aug 10, 2017 at 4:03 PM, Violet Cheng <vcheng(at)pivotal(dot)io> wrote:
>
>> 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
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> ​
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>