Thanks patch applied. On Wed, Aug 16, 2017 at 11:30 AM, Violet Cheng <vch...@pivotal.io> wrote:
> Thanks Surinder! Hope it could be committed soon :) > > On Wed, Aug 16, 2017 at 1:34 PM, Surinder Kumar < > surinder.ku...@enterprisedb.com> wrote: > >> Hi Violet, >> >> I have already reviewed this patch. Here is the link >> <https://www.postgresql.org/message-id/CAM5-9D8UJVgCD0MrKq6yPx0qx9k8v3r6_4e8SkHdGV1RBDnhbQ%40mail.gmail.com> >> . >> >> Thanks, >> Surinder >> >> On Wed, Aug 16, 2017 at 8:57 AM, Violet Cheng <vch...@pivotal.io> wrote: >> >>> Hi, >>> >>> Any update on this patch? Could it be committed soon? >>> >>> Thanks, >>> Violet >>> >>> On Fri, Aug 11, 2017 at 1:40 PM, Sarah McAlear <smcal...@pivotal.io> >>> wrote: >>> >>>> 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 <vch...@pivotal.io> >>>> wrote: >>>> >>>>> Here's the Redmine link >>>>> >>>>> https://redmine.postgresql.org/issues/2644 >>>>> >>>>> On Thu, Aug 10, 2017 at 4:03 PM, Violet Cheng <vch...@pivotal.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.ku...@enterprisedb.com> wrote: >>>>>> >>>>>>> Please find attached screenshot. >>>>>>> >>>>>>> On Thu, Aug 10, 2017 at 11:30 AM, Surinder Kumar < >>>>>>> surinder.ku...@enterprisedb.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.ku...@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> The updated patch looks good to me. >>>>>>>>> >>>>>>>>> On Wed, Aug 9, 2017 at 4:15 PM, Sarah McAlear <smcal...@pivotal.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 < >>>>>>>>>> smcal...@pivotal.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.ku...@enterprisedb.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Wenlin, >>>>>>>>>>>> >>>>>>>>>>>> On Tue, Aug 8, 2017 at 3:15 PM, Wenlin Zhang <wzh...@pivotal.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.ku...@enterprisedb.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 < >>>>>>>>>>>>>> wzh...@pivotal.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.va...@enterprisedb.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 < >>>>>>>>>>>>>>>> smcal...@pivotal.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 < >>>>>>>>>>>>>>>>> wzh...@pivotal.io> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Hi Hackers, >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> This patch changes the alert style in the sub-navigation >>>>>>>>>>>>>>>>>> to match style guide. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>> Wenlin, Shirley & Sarah >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > -- *Akshay Joshi* *Principal Software Engineer * *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
