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