Awesome, thanks! Akshay Joshi <akshay.jo...@enterprisedb.com>于2017年8月17日 周四下午8:24写道：
> 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* >
