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