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

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