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