Обсуждение: [pgAdmin4][Patch]: Refactor of the History Tab
Hi Hackers,
Attached is the patch that refactors the react components that supports the history detail pane. We moved inline styling. whenever possible, to scss.
Thanks,
Joao and Shruti
Вложения
Surinder,
It would be great if you could take a look at this an see if the style changes are more inline with what you were thinking when you commented on the first patch.
-- Rob
On Fri, Jun 30, 2017 at 11:55 AM, Shruti Iyer <siyer@pivotal.io> wrote:
Hi Hackers,Attached is the patch that refactors the react components that supports the history detail pane. We moved inline styling. whenever possible, to scss.Thanks,Joao and Shruti
Hi,
Review comments:
1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.
2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?
3) No query execution message appears if i run same query second time, please refer screenshot.
Apart from this patch looks good.
Thanks,
Surinder
On Fri, Jun 30, 2017 at 9:58 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:
Surinder,It would be great if you could take a look at this an see if the style changes are more inline with what you were thinking when you commented on the first patch.-- RobOn Fri, Jun 30, 2017 at 11:55 AM, Shruti Iyer <siyer@pivotal.io> wrote:Hi Hackers,Attached is the patch that refactors the react components that supports the history detail pane. We moved inline styling. whenever possible, to scss.Thanks,Joao and Shruti
Вложения
Hi,
On Fri, Jun 30, 2017 at 10:30 PM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi,Review comments:1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?3) No query execution message appears if i run same query second time, please refer screenshot.
I think this isn't related to this patch, just now I have sent patch for similar issue.
Apart from this patch looks good.Thanks,SurinderOn Fri, Jun 30, 2017 at 9:58 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:Surinder,It would be great if you could take a look at this an see if the style changes are more inline with what you were thinking when you commented on the first patch.-- RobOn Fri, Jun 30, 2017 at 11:55 AM, Shruti Iyer <siyer@pivotal.io> wrote:Hi Hackers,Attached is the patch that refactors the react components that supports the history detail pane. We moved inline styling. whenever possible, to scss.Thanks,Joao and Shruti
Hi,Review comments:1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.
No, the style guide needs an entry for monospaced fonts which are very much needed in a few places like this.
2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?3) No query execution message appears if i run same query second time, please refer screenshot.Apart from this patch looks good.Thanks,SurinderOn Fri, Jun 30, 2017 at 9:58 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:Surinder,It would be great if you could take a look at this an see if the style changes are more inline with what you were thinking when you commented on the first patch.-- RobOn Fri, Jun 30, 2017 at 11:55 AM, Shruti Iyer <siyer@pivotal.io> wrote:Hi Hackers,Attached is the patch that refactors the react components that supports the history detail pane. We moved inline styling. whenever possible, to scss.Thanks,Joao and Shruti
<Screen Shot 2017-06-30 at 10.27.33 pm.png>
Hi
2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?
It sounds like you're describing the distinction between the query history entries and the Messages, History, etc. tab bar (screenshot attached).
If so, the gray/blue contrast should be sufficient to differentiate.
Thanks,
Shirley and George
Вложения
1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.
As per Dave's comment, we have added this to the styleguide backlog. We will be adding monospace entries to the styleguide.
2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?
We have updated the patch to include a 1px solid #cccccc border at the top.
3) No query execution message appears if i run same query second time, please refer screenshot.
Appears to be fixed with Murtuza's patch.
Thanks,
Sarah and Matt
On Mon, Jul 3, 2017 at 10:05 AM, George Gelashvili <ggelashvili@pivotal.io> wrote:
Hi2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?It sounds like you're describing the distinction between the query history entries and the Messages, History, etc. tab bar (screenshot attached).If so, the gray/blue contrast should be sufficient to differentiate.Thanks,Shirley and George
Вложения
Hi
--
On Wed, Jul 5, 2017 at 2:54 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.As per Dave's comment, we have added this to the styleguide backlog. We will be adding monospace entries to the styleguide.2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?We have updated the patch to include a 1px solid #cccccc border at the top.3) No query execution message appears if i run same query second time, please refer screenshot.Appears to be fixed with Murtuza's patch.
I get the following error with this patch when bundling:
[235] ./scss/pgadmin.scss 1.1 kB {3} [built] [failed] [1 error]
+ 363 hidden modules
ERROR in ./scss/pgadmin.scss
Module build failed:
@import 'primaryblue';
^
File to import not found or unreadable: primaryblue.
Parent style sheet: stdin
in /Users/dpage/git/pgadmin4/web/pgadmin/static/scss/pgadmin.scss (line 4, column 1)
ERROR in ./scss/pgadmin.scss
Module build failed: ModuleBuildError: Module build failed:
@import 'primaryblue';
^
File to import not found or unreadable: primaryblue.
Parent style sheet: stdin
in /Users/dpage/git/pgadmin4/web/pgadmin/static/scss/pgadmin.scss (line 4, column 1)
at runLoaders (/Users/dpage/git/pgadmin4/web/node_modules/webpack/lib/NormalModule.js:192:19)
at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib/LoaderRunner.js:364:11
at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib/LoaderRunner.js:230:18
at context.callback (/Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
at Object.asyncSassJobQueue.push [as callback] (/Users/dpage/git/pgadmin4/web/node_modules/sass-loader/lib/loader.js:55:13)
at Object.<anonymous> (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async.js:2243:31)
at Object.callback (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async.js:906:16)
at options.error (/Users/dpage/git/pgadmin4/web/node_modules/node-sass/lib/index.js:294:32)
Child extract-text-webpack-plugin:
[0] /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/pgadmin4/web/~/sass-loader/lib/loader.js!./scss/pgadmin.scss 247 bytes {0} [built] [failed] [1 error]
ERROR in /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/pgadmin4/web/~/sass-loader/lib/loader.js!./scss/pgadmin.scss
Module build failed:
@import 'primaryblue';
^
File to import not found or unreadable: primaryblue.
Parent style sheet: stdin
in /Users/dpage/git/pgadmin4/web/pgadmin/static/scss/pgadmin.scss (line 4, column 1)
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Sorry Dave. It looks like we had formatted the patch incorrectly. We have recreated the patch.
Matt and Sarah
On Wed, Jul 5, 2017 at 10:49 AM, Dave Page <dpage@pgadmin.org> wrote:
HiOn Wed, Jul 5, 2017 at 2:54 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.As per Dave's comment, we have added this to the styleguide backlog. We will be adding monospace entries to the styleguide.2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?We have updated the patch to include a 1px solid #cccccc border at the top.3) No query execution message appears if i run same query second time, please refer screenshot.Appears to be fixed with Murtuza's patch.I get the following error with this patch when bundling:[235] ./scss/pgadmin.scss 1.1 kB {3} [built] [failed] [1 error]+ 363 hidden modulesERROR in ./scss/pgadmin.scssModule build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinin /Users/dpage/git/pgadmin4/web/pgadmin/static/scss/pgadmin. scss (line 4, column 1) ERROR in ./scss/pgadmin.scssModule build failed: ModuleBuildError: Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinin /Users/dpage/git/pgadmin4/web/pgadmin/static/scss/pgadmin. scss (line 4, column 1) at runLoaders (/Users/dpage/git/pgadmin4/web/node_modules/webpack/lib/ NormalModule.js:192:19) at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/ lib/LoaderRunner.js:364:11 at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/ lib/LoaderRunner.js:230:18 at context.callback (/Users/dpage/git/pgadmin4/web/node_modules/loader- runner/lib/LoaderRunner.js: 111:13) at Object.asyncSassJobQueue.push [as callback] (/Users/dpage/git/pgadmin4/web/node_modules/sass-loader/ lib/loader.js:55:13) at Object.<anonymous> (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/ async.js:2243:31) at Object.callback (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/ async.js:906:16) at options.error (/Users/dpage/git/pgadmin4/web/node_modules/node-sass/ lib/index.js:294:32) Child extract-text-webpack-plugin:[0] /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/ lib/loader.js!./scss/pgadmin. scss 247 bytes {0} [built] [failed] [1 error] ERROR in /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/ lib/loader.js!./scss/pgadmin. scss Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinin /Users/dpage/git/pgadmin4/web/pgadmin/static/scss/pgadmin. scss (line 4, column 1) --Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Thanks, applied.
On Wed, Jul 5, 2017 at 4:31 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
Sorry Dave. It looks like we had formatted the patch incorrectly. We have recreated the patch.Matt and SarahOn Wed, Jul 5, 2017 at 10:49 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Jul 5, 2017 at 2:54 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.As per Dave's comment, we have added this to the styleguide backlog. We will be adding monospace entries to the styleguide.2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?We have updated the patch to include a 1px solid #cccccc border at the top.3) No query execution message appears if i run same query second time, please refer screenshot.Appears to be fixed with Murtuza's patch.I get the following error with this patch when bundling:[235] ./scss/pgadmin.scss 1.1 kB {3} [built] [failed] [1 error]+ 363 hidden modulesERROR in ./scss/pgadmin.scssModule build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinERROR in ./scss/pgadmin.scssModule build failed: ModuleBuildError: Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinat runLoaders (/Users/dpage/git/pgadmin4/web/node_modules/webpack/lib/Norm alModule.js:192:19) at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib /LoaderRunner.js:364:11 at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib /LoaderRunner.js:230:18 at context.callback (/Users/dpage/git/pgadmin4/web/node_modules/loader-runner/ lib/LoaderRunner.js:111:13) at Object.asyncSassJobQueue.push [as callback] (/Users/dpage/git/pgadmin4/web/node_modules/sass-loader/lib/ loader.js:55:13) at Object.<anonymous> (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async .js:2243:31) at Object.callback (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async .js:906:16) at options.error (/Users/dpage/git/pgadmin4/web/node_modules/node-sass/lib/ index.js:294:32) Child extract-text-webpack-plugin:[0] /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/lib /loader.js!./scss/pgadmin.scss 247 bytes {0} [built] [failed] [1 error] ERROR in /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/lib /loader.js!./scss/pgadmin.scss Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdin--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
And, Houston, we have a problem. The layout is quite broken on IE - which of course we're working to use on Windows. Can you take a look ASAP please Matt & friends?
Screenshot attached.
On Thu, Jul 6, 2017 at 1:08 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks, applied.On Wed, Jul 5, 2017 at 4:31 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:Sorry Dave. It looks like we had formatted the patch incorrectly. We have recreated the patch.Matt and SarahOn Wed, Jul 5, 2017 at 10:49 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Jul 5, 2017 at 2:54 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.As per Dave's comment, we have added this to the styleguide backlog. We will be adding monospace entries to the styleguide.2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?We have updated the patch to include a 1px solid #cccccc border at the top.3) No query execution message appears if i run same query second time, please refer screenshot.Appears to be fixed with Murtuza's patch.I get the following error with this patch when bundling:[235] ./scss/pgadmin.scss 1.1 kB {3} [built] [failed] [1 error]+ 363 hidden modulesERROR in ./scss/pgadmin.scssModule build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinERROR in ./scss/pgadmin.scssModule build failed: ModuleBuildError: Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinat runLoaders (/Users/dpage/git/pgadmin4/web/node_modules/webpack/lib/Norm alModule.js:192:19) at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib /LoaderRunner.js:364:11 at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib /LoaderRunner.js:230:18 at context.callback (/Users/dpage/git/pgadmin4/web/node_modules/loader-runner/li b/LoaderRunner.js:111:13) at Object.asyncSassJobQueue.push [as callback] (/Users/dpage/git/pgadmin4/web/node_modules/sass-loader/lib/ loader.js:55:13) at Object.<anonymous> (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async .js:2243:31) at Object.callback (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async .js:906:16) at options.error (/Users/dpage/git/pgadmin4/web/node_modules/node-sass/lib/in dex.js:294:32) Child extract-text-webpack-plugin:[0] /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/lib /loader.js!./scss/pgadmin.scss 247 bytes {0} [built] [failed] [1 error] ERROR in /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/lib /loader.js!./scss/pgadmin.scss Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdin--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Working on it.
-- Rob
On Thu, Jul 6, 2017 at 10:03 AM, Dave Page <dpage@pgadmin.org> wrote:
And, Houston, we have a problem. The layout is quite broken on IE - which of course we're working to use on Windows. Can you take a look ASAP please Matt & friends?Screenshot attached.On Thu, Jul 6, 2017 at 1:08 PM, Dave Page <dpage@pgadmin.org> wrote:Thanks, applied.On Wed, Jul 5, 2017 at 4:31 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:Sorry Dave. It looks like we had formatted the patch incorrectly. We have recreated the patch.Matt and SarahOn Wed, Jul 5, 2017 at 10:49 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, Jul 5, 2017 at 2:54 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:1) I see the font used for class .query-history .entry is monospace, shouldn't it be Helvetica as per style guide ? the font for Messages > text is also monospace.As per Dave's comment, we have added this to the styleguide backlog. We will be adding monospace entries to the styleguide.2) Can 1px top border be added above first entry in left panel to differentiate from above panels ?We have updated the patch to include a 1px solid #cccccc border at the top.3) No query execution message appears if i run same query second time, please refer screenshot.Appears to be fixed with Murtuza's patch.I get the following error with this patch when bundling:[235] ./scss/pgadmin.scss 1.1 kB {3} [built] [failed] [1 error]+ 363 hidden modulesERROR in ./scss/pgadmin.scssModule build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinERROR in ./scss/pgadmin.scssModule build failed: ModuleBuildError: Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdinat runLoaders (/Users/dpage/git/pgadmin4/web/node_modules/webpack/lib/Norm alModule.js:192:19) at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib /LoaderRunner.js:364:11 at /Users/dpage/git/pgadmin4/web/node_modules/loader-runner/lib /LoaderRunner.js:230:18 at context.callback (/Users/dpage/git/pgadmin4/web/node_modules/loader-runner/li b/LoaderRunner.js:111:13) at Object.asyncSassJobQueue.push [as callback] (/Users/dpage/git/pgadmin4/web/node_modules/sass-loader/lib/ loader.js:55:13) at Object.<anonymous> (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async .js:2243:31) at Object.callback (/Users/dpage/git/pgadmin4/web/node_modules/async/dist/async .js:906:16) at options.error (/Users/dpage/git/pgadmin4/web/node_modules/node-sass/lib/in dex.js:294:32) Child extract-text-webpack-plugin:[0] /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/lib /loader.js!./scss/pgadmin.scss 247 bytes {0} [built] [failed] [1 error] ERROR in /Users/dpage/git/pgadmin4/web/~/css-loader!/Users/dpage/git/ pgadmin4/web/~/sass-loader/lib /loader.js!./scss/pgadmin.scss Module build failed:@import 'primaryblue';^File to import not found or unreadable: primaryblue.Parent style sheet: stdin--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company