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