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