Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aoZ8F-0002qp-Ps for pgadmin-hackers@arkaria.postgresql.org; Fri, 08 Apr 2016 16:19:40 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1aoZ8E-0001Q4-1N for pgadmin-hackers@arkaria.postgresql.org; Fri, 08 Apr 2016 16:19:38 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1aoZ8D-0001Pr-8B for pgadmin-hackers@postgresql.org; Fri, 08 Apr 2016 16:19:37 +0000 Received: from mail-ig0-x22e.google.com ([2607:f8b0:4001:c05::22e]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1aoZ88-0002ok-4r for pgadmin-hackers@postgresql.org; Fri, 08 Apr 2016 16:19:36 +0000 Received: by mail-ig0-x22e.google.com with SMTP id f1so16814337igr.1 for ; Fri, 08 Apr 2016 09:19:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=XuUAFhv4DZxgLGN/gASxAMAzUgVl+xQrpaWqotMMXzA=; b=DcoOFVKZiN2V8+fP2Hq1O2t/o8/zwZQ+R2YiOLeigUaODXLqbOJsJ31ffMsId4p/A5 HTsBFRZV4JiaEUx0N4ze3m086euD4kP47HWg4JHZXbXjR1+S0iZKiRsJDapwY5dj+VSW LYqiOFVdny81RrdZhkAqtsghtbIfUFHN/RQWqFateWxAK8MC3jUkOZfisypZFxR7DdJC PFeFAaJ/mbFvq7xk2zDw0ZZazuz80k0Odg0oA2jam7rnfHaerLgvGHIjTUHtPKZvpK3T /DCZgHafj+6tLdu6dSUFtTDvsnoVkiu4kPKHN5l3IIxkUojG8c5SGwUrQ8tmNyGLoo9G 0dtw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=XuUAFhv4DZxgLGN/gASxAMAzUgVl+xQrpaWqotMMXzA=; b=YWmshpiC6/9UcTJm/3fy+HAVmzlTCsWfg95jWaXVfsscxqlRb+zSjqD7wrmU5u3wwI 4/vj+zUpQnn3ksqdJdR+/6H0mPKDcV9H56R7ExRG95VDL8j/jhcM+2Eg8N2NzZ5rg3s5 lNNKCumNsn/W6bp1U9adKaNLZ7DKAsHyjSTv3/5gwMQ57+VT1sO0zdZIrwSeAzqnfXzZ q89OajxDQBUZtxwbL/9GHJze8VS6HLXJovye2/z0q+/VT3eot38MiPRbKxSVQSzbANPL JI6D54xQaK8LozA8FjKd2nKsHJNADW6mQDO673bgyapNVnvIKAu3kjROkggClQ7+X5PY GAtw== X-Gm-Message-State: AD7BkJK40IN2E+dovHrySqmVfYHCIOEOMvwqFA1MZGeH1yApdJkeII9lEWd+UXyZeNWh2saZSGbwCU8g+YDrqw== MIME-Version: 1.0 X-Received: by 10.50.66.171 with SMTP id g11mr4649441igt.96.1460132369853; Fri, 08 Apr 2016 09:19:29 -0700 (PDT) Received: by 10.64.105.131 with HTTP; Fri, 8 Apr 2016 09:19:29 -0700 (PDT) In-Reply-To: References: Date: Fri, 8 Apr 2016 17:19:29 +0100 Message-ID: Subject: Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool From: Dave Page To: Akshay Joshi Cc: pgadmin-hackers Content-Type: multipart/related; boundary=047d7bdc08a63badca052ffb8f8e X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --047d7bdc08a63badca052ffb8f8e Content-Type: multipart/alternative; boundary=047d7bdc08a63badc6052ffb8f8d --047d7bdc08a63badc6052ffb8f8d Content-Type: text/plain; charset=UTF-8 On Fri, Apr 8, 2016 at 3:39 PM, Akshay Joshi wrote: > Hi Dave > > On Fri, Apr 8, 2016 at 2:32 PM, Dave Page wrote: > >> >> >> On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi < >> akshay.joshi@enterprisedb.com> wrote: >>> >>> >>>>>>>> - The Query Tool menu icon should be a glyphicon, to match the >>>>>>>> others. >>>>>>>> >>>>>>> >>>>>>> There is no glyphicon available which match the Query Tool icon. >>>>>>> I have found one like below which is "database-search" or can you please >>>>>>> suggest some other icon. >>>>>>> [image: Inline image 1] >>>>>>> >>>>>> >>>>>> That one looks perfect. >>>>>> >>>>> >>> We can't use this icon because it's not come with Bootstrap , >>> I have picked this from "http://glyphicons.com/" and I am not sure we >>> can use it as per the Licence. >>> >> >> At the risk of annoying everyone immensely, on reflection I'm thinking we >> should use Font Awesome as our default generic icons, and fall back to >> Bootstrap's Glyphicons. I really hadn't realised how much larger the FA set >> is. >> >> For this particular issue, could we use FA's stacking? e.g. something >> like: https://jsfiddle.net/pa8x6nt3/ >> >> If not, how about using the execute icon we discussed, e.g. fa-bolt? >> > > I have used fa-bolt. Apart from that I have completed below review > comments > OK. > > > - The View Data menu option should be on the Object menu, which should > mirror the Context menu, except options should be disabled when not > applicable instead of hidden. *Note*: - With current implementation > "Object" menu is re-created dynamically depending on the node clicked, so > we will have to re-create the "View Data" menu as well and it require > change in the generic code. For the time being "View Data" menu is visible > in "Object" menu when appropriate node is selected. > - Please merge the functionality of the Refresh and Execute buttons > into one button. We shouldn't have two buttons that do essentially the same > thing. > - In Edit Grid mode, that textbox should be read-only, but should > display the SQL used (including any LIMIT/FILTER clauses). Please > refer the attached screenshot (Modified-Data-Grid). > - Please adjust the height of the Delete icon in the Edit Grid, such > that it doesn't force the row height to be higher than it should be. > - I think the names of the tabs are far too long. Can we change them > to "Query 1", "Query 2" etc, then rename them to the filename if the > user saves/loads a file? *Note*: - As discussed I have added one > more container to show the title. Please refer the attached screenshots > (Modified-Query-Tool) > - Query results should have spaces converted to " ", so that > proper indenting is maintained (for example, on EXPLAIN queries) > - To fix this we have added css style "*white-space: pre-wrap;*", but > it changes the backgrid cell size. Please refer the screenshot > (Modified-Data-Grid). > > Hmmm. I'd rather that the heights didn't change (or at least the rows were resizeable like in pga3. Can we set the column to hide vertical overflow, unless it gets focus or something? > Please review the screenshots and please let me know will it looks good. > Bar the cell size, certainly it's good :-) -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --047d7bdc08a63badc6052ffb8f8d Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Fri, Apr 8, 2016 at 3:39 PM, Akshay Joshi <akshay.joshi= @enterprisedb.com> wrote:
<= div dir=3D"ltr">Hi Dave

On Fri, Apr 8, 2016 at 2:32 PM, Dave Page <d= page@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi <ak= shay.joshi@enterprisedb.com> wrote:
<= div class=3D"gmail_quote">

- The Query Tool menu icon should be a glyphico= n, to match the others.
=C2=A0
=C2=A0 =C2=A0There is no glyphicon available whi= ch match the Query Tool icon. I have found one like below which is "da= tabase-search" or can you please suggest some other icon.=C2=A0
<= div>=C2=A0 =C2=A0 =C2=A0 =C2=A03D"==C2= =A0

That one= looks perfect.

=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0We can't use this icon be= cause it's not come with Bootstrap , I have picked this from "http://glyphicons.com/= " and I am not sure we can use it as per the Licence.

At the risk of annoying ever= yone immensely, on reflection I'm thinking we should use Font Awesome a= s our default generic icons, and fall back to Bootstrap's Glyphicons. I= really hadn't realised how much larger the FA set is.
For this particular issue, could we use FA's stacking? e.g= . something like:=C2=A0https://jsfiddle.net/pa8x6nt3/=C2=A0

= If not, how about using the execute icon we discussed, e.g. fa-bolt?
<= /div>

=C2=A0 =C2=A0I hav= e used fa-bolt. Apart from that I have completed below review comments=C2= =A0

OK.
= =C2=A0

  • The View D= ata menu option should be on the Object menu, which should mirror the Conte= xt menu, except options should be disabled when not applicable instead of h= idden. Note: - With current implementation "Object" menu i= s re-created dynamically depending on the node clicked, so we will have to = re-create the "View Data" menu as well and it require change in t= he generic code. For the time being "View Data" menu is visible i= n "Object" menu when appropriate node is selected. =C2=A0 =C2=A0 = =C2=A0=C2=A0
  • Please merge the functionality of the Refresh and Execute buttons into o= ne button. We shouldn't have two buttons that do essentially the same t= hing.
  • In Edit Grid mode, that textbox should = be read-only, but should display the SQL used (including any L= IMIT/FILTER clauses). Please refer the attached screenshot (Modified-Data-G= rid).=C2=A0
  • Please adjust the height of the Delete icon in the Edit Grid, such that= it doesn't force the row height to be higher than it should be.=
  • I think = the names of the tabs are far too long. Can we change them to "Query 1= ", "Query 2" etc, then rename them to the filename if the user saves/loads a file? No= te: - As discussed I have added one mo= re=C2=A0container=C2=A0to show the title. Please refer the attached screens= hots (Modified-Query-Tool)=C2=A0
  • Query results s= hould have spaces converted to "&nbsp;", so that= proper indenting is maintained (for example, on EXPLAIN queries)
    • To fix this we have added css style "<= /span>white-space: pre-wrap;", but it changes the = backgrid cell size. Please refer the screenshot=C2=A0(Modified-Data-Grid).=C2=A0
    =
Hmmm. I'd rather that th= e heights didn't change (or at least the rows were resizeable like in p= ga3. Can we set the column to hide vertical overflow, unless it gets focus = or something?
=C2=A0
=C2=A0 =C2=A0Please review the screenshots and pl= ease let me know will it looks good.

Bar the cell size, certainly it's good :-)=C2= =A0

--
Dave Pa= ge
Blog: http:= //pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterpris= edb.com
The Enterprise PostgreSQL Company
--047d7bdc08a63badc6052ffb8f8d-- --047d7bdc08a63badca052ffb8f8e Content-Type: image/png; name="image.png" Content-Disposition: inline; filename="image.png" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: ii_153efa7c0422527f iVBORw0KGgoAAAANSUhEUgAAAGYAAABkCAYAAAB0F0VpAAAJMUlEQVR4Ae2cR4gUTRTH3wZzzjnn nHNCPAgiHgwHFVFBUBEDCC6CyKfgQfGkBxEzhosHj6KgKKKIiGIOmDCnNee0H7+C6q+nd2d3Zqdm 5rlfPSi6p6b6dfX/3/XqvVc1k1NYWFgkXtQhkKuuR75DBgFPjNIXwRPjiVGKgNJu+RHjiVGKgNJu +RHjiVGKgNJu+RHjiVGKgNJu+RHjiVGKgNJu+RHjiVGKgNJu+RHjiVGKgNJu+RHjiVGKgNJu+RHj iVGKgNJu+RHjiVGKgNJu+RHjiVGKgNJu+RHjiVGKgNJu+RHjiVGKgNJu+RGjlJh8pf0KuvXlyxd5 8OCBPHz4UD5+/CifPn0y5fPnz0KhjjZI9erVpWbNmqbUqFFDKLVq1TKldevW0rZtW9MmUK74JEfj zzCePHkiZ8+elfPnz8uzZ89KhA8S6tSpI9WqVTOFRl+/fjXl/fv3AVnRi5s1ayYDBw6UoUOHSosW LaJfq/msipi3b9/K/v37DSElIdS4cWOZNm2adO7cWZo0aSI5OTmmhNsWFRUJ5cWLF3L79m05ePCg vHz5MtwkOIegmTNnSr169YI6LSeq5pgtW7bEJQXAfv/+Lbm5uWaEQAry58+fmEId3zGSaMs18YQR yT01iqo5hje8NCksLJTNmzebJoDO/MGcgklDMGHMQcw7EJaIlHXPRHSko40qYpJ5QICHCApzUkUT VaasooGbyvN4YlJBL43XemLSCG4qqj0xqaCXxmtVEdOtW7c0PmrJqrNxz5J7Eluriphly5aZiDy2 i+n7RPTPPTWKqsi/atWqkpeXJ48ePZLTp0+btIxrV5g0DISMGDFCWrVqZQLQb9++qeNGFTGHDx82 oLVs2TIAikTl/fv3Tfnw4UOQyCSItElMgkrEJjA5EnySxKxdu7Yp7dq1Ewp1VghYT548KePHj7dV ao6qiJkzZ44Bpnfv3uaN7tmzp5Afcylv3ryRy5cvy7lz5+TSpUvy48cP2b17t8tbONGlMvIHOApC grFTp04madmoUSNp0KCB1K9f34wCMsxVqlQx5o+25MW+f/9uMsuMIpKiEEFC8+nTp2b54NWrV06A S7cSlcSEHxpwebv/b6KemFQJYf2FEUZhtFWqVEnevXsnr1+/lps3b5r1m1TvkY7rKyQxzZs3l1Gj Rsno0aNLnaN+/fplyEkHsKnqrFDE4IktWLBABg8eHOBCFtountmlANZrWDbANcfBwLvTJqqIwZ3F NS6P9OjRQxYvXmzMFQQwGpjo7927Z3SybwBh3d+6zjgWEKRRVLnLeFR79+6VM2fOJLzQBah9+/aV lStXmpVLdOCRsaR8/PjxUjEfN26cTJ06NdgzUGrjDH+pihi7HMzEfOTIERNnsDumtOVhVi83btxo AkpIuXbtmuzatcuMlkSwxAWfO3eudO/ePZHmGWujihjIYNIGLCsEgJi3O3fumHiE6B+viiOrl4sW LZI+ffoIaZXr16/Lhg0b7KXBkSC1Y8eOUrlyZXn+/LncunXLzDtBAxFZsWKFKnJUEUPkn5+fb7YX 9e/f35iosnawMLGzbYm9ZatXr44ZKWQQ5s+fX8wzY9fMnj17YuIjXoa1a9eqMWvqiAm/xXhPvO0k HinEJJguWwDz58+fxtQxNx07diy4fPr06TJ58mTzGWcg7JHhjWEeN23aZOYzexFzzqxZs+zHrB5V eWVRJOz+MFIqFy5ciH5tth4xL5EdCJPCSIEUm6JBT1ggHLO2ZMkS473ZzAI6Jk6cqGKfmU5fMYxi nHMieUhBoi425ou5iXknSgrtqcNR4Dh79uyYTYNRXXFun/bqv5aY8NKAjVFAC9NH6gUTV5ZADgR3 6dIlaBrWFVRm4aTCEYP3BeCJCiOradOmQXNPTADFfydE5IkK6X4rzBlWmDtKMl/2++iRtuH2WlYz VY2YgoICGTlyZEJpEuIRK+3bt7encX8dEDQo4SSsC89Pg6gihphl6dKlZn/ypEmTpEOHDnFJCoNJ OyvsRSZzkKjQNrx/OWzWEtWRjnaq3OVDhw7JsGHDhLS9jSeYL4j68ZZYo2dFkhI2OSQmrWCWDhw4 YDICxCulCe40ba0pwySyUUODqAswyfaSlBw+fLh07drVLCmXBBQBIxE/AgH79u0z+TXbltQ/bnM8 ciBl69atMdE/pJChTsSjs/dJ11HViOEhAZxg0gaUmDdMFT9UwhUm2ifyZ/cL6RtiGa7hB0gXL14M fqRE0Ejqf8aMGdKwYcMY/DBfjBR7D/tlmzZtzDX2czaP6kZMMmDgga1bt86QxTnJyTVr1sSowDzh QkMsQhYB02jNV0xjEZNpHjNmTLQ645//amJAq1evXrJ8+XIDHKOHDDO/Eov3875EEGYZINvkqPLK EgEt2ubKlStm1yb1OAosFbM+U9YmPkbSkCFDZMqUKVGVZj3nxIkTxeozWaFujinPw+/cuVPYXjtg wADjEBB8zps3zyQy7969K7bgyeEOU6z3x/3q1q0rO3bsiLk1mwAxd2PHjo2pz9QHVaZs/fr1cuPG jXI9O97XwoULzVoOCphz2KpUmuAc4DjQlkW67du3F2tOkjMb5OQVFBT8U6w3WaoYNGiQCQ4fP36c dA94u/HE8Lj4uTlEATxuMeAjtOGcesyeJYZ6Epl4e1FPjW206AuvqibduXJcoIoYNoOzCx8zw7nd RJ7Mc7FH4NSpU4YYMseYNUtGlCirl+/jkUO6Z8KECWWOPqvL1VGVKWNCxqwQn1hJdrc/+5nDf13S r18/sycgkW1K3Bcijx49Ktu2bTNu9qpVq0xayI462690H1URYx8WggCJkgig9jp75O0nnY8Z4zwZ seRcvXpVGC0QFU7/JKMrlbYqiYk+EOQwZ0AYwjFcwu0hIlwgh5KMWHK4Lhuk0Nf/bEYyPc9wW8xI Jk2JdQoyec8opH99gBl9IFefs0kKz+CJccWkYz2eGMeAulLniXGFpGM9nhjHgLpS54lxhaRjPZ4Y x4C6UueJcYWkYz2eGMeAulLniXGFpGM9nhjHgLpS54lxhaRjPZ4Yx4C6UueJcYWkYz2eGMeAulLn iXGFpGM9nhjHgLpS54lxhaRjPZ4Yx4C6UueJcYWkYz2eGMeAulLniXGFpGM9nhjHgLpS9y9BjyDx r+DRaQAAAABJRU5ErkJggg== --047d7bdc08a63badca052ffb8f8e--