Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1apxcu-000396-I4 for pgadmin-hackers@arkaria.postgresql.org; Tue, 12 Apr 2016 12:41:04 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1apxcu-0005Yf-4k for pgadmin-hackers@arkaria.postgresql.org; Tue, 12 Apr 2016 12:41:04 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1apxct-0005YZ-8E for pgadmin-hackers@postgresql.org; Tue, 12 Apr 2016 12:41:03 +0000 Received: from mail-io0-x232.google.com ([2607:f8b0:4001:c06::232]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1apxcl-0006Me-Hw for pgadmin-hackers@postgresql.org; Tue, 12 Apr 2016 12:41:02 +0000 Received: by mail-io0-x232.google.com with SMTP id g185so26002167ioa.2 for ; Tue, 12 Apr 2016 05:40:55 -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=dUfFTOuQcueYpLeFV9N8E7juOj0LuLEMGsR4mbcRJzk=; b=HsnIYk2y6iWvaqlYltmys1VYjfcX7Y0r6g158Hqgh66pdSw8gy674Hn4zSuPBBFrIp me2WiHcgoQEIR5pX2tOU6Dck2lSPvvUqTk0pA1/OAsGOIIcJi+EZRMEpByIRpUs2K6XF DffN6OSrvTSO2iRDznjUsewBzPHahJJZ6HoysGLOjj0jNd+qh1Cgh+2NPYPH4N5O/hlk otjgEFf4WHD+llmjT7F3+Tr7TAzJJ5RLliG4ip1Utzb36S6neqvk6apHziNX9fEgIq40 vj91bLJckeahvb5G+74Q8Xb2INmEs++4Q5VeKhytELqctz4r4Gx/Ye9wRJQLGpmnQeFn uShQ== 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=dUfFTOuQcueYpLeFV9N8E7juOj0LuLEMGsR4mbcRJzk=; b=KllBtSfRZx5P33eokhdRtHbm82A+peNaH/LaA8i5Zgv2f5b5FIUxrV9k3J9UsLWtcH s2TFmJ2Z4Z97HQ9tyQKn9pJOMahA6INuCcxotSO4zztMhvoWDCWBpCz+9gy5miQHGaKa IJ2KoXL2BKOa4bh/GVQPBbcVMBtcoaNrD+FR/mehvj6azAeSAnuiCIXyQ82t/HKbu8LU 6CEkQiHmu6bCclGi6G5k+UfWuQmLSq0BXjthuXA/MpZwKcMGHU/nC4zQp0t3nN5TcnBJ eAhQxkw1KcvktWyjOZ18AT/cGm1InodOqvwoEw71QlqMpEN1jgyC0DSvf4pal+pmN5SK vT6g== X-Gm-Message-State: AOPr4FX3KE4cBNBIBEGVHWUdZoKX8RyEpTgqDndZig6mnUh675ifxnvT7iVNVLZUPo0fH4aMktFuNGM/58/0uw== MIME-Version: 1.0 X-Received: by 10.107.56.196 with SMTP id f187mr3691769ioa.156.1460464854285; Tue, 12 Apr 2016 05:40:54 -0700 (PDT) Received: by 10.64.105.131 with HTTP; Tue, 12 Apr 2016 05:40:54 -0700 (PDT) In-Reply-To: References: Date: Tue, 12 Apr 2016 13:40:54 +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=001a114ab7b0d8f828053048f8ad 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 --001a114ab7b0d8f828053048f8ad Content-Type: multipart/alternative; boundary=001a114ab7b0d8f824053048f8ac --001a114ab7b0d8f824053048f8ac Content-Type: text/plain; charset=UTF-8 Very nice! On Tue, Apr 12, 2016 at 1:11 PM, Akshay Joshi wrote: > Hi Dave > > As per suggestion, I have changed History tab to be a grid. Attached is > the two screenshot one for Query Tool and other is for the Data Grid where > some of the transaction gets rollback while saving the data due to one of > the error condition. Please review the screenshots and let me know the > comments if any. > > On Fri, Apr 8, 2016 at 9:49 PM, Dave Page wrote: > >> >> >> On Fri, Apr 8, 2016 at 3:39 PM, Akshay Joshi < >> akshay.joshi@enterprisedb.com> 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 >> > > > > -- > *Akshay Joshi* > *Principal Software Engineer * > > > > *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246* > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --001a114ab7b0d8f824053048f8ac Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Very nice!

On Tue, Apr 12, 2016 at 1:11 PM, Akshay Joshi <a= kshay.joshi@enterprisedb.com> wrote:
Hi Dave

As per suggestion, I = have changed=C2=A0History tab to be a grid= . Attached is the two screenshot one for Query Tool and other is for the Da= ta Grid where some of the transaction gets rollback while saving the data d= ue to one of the error condition. Please review the screenshots and let me = know the comments if any. =C2=A0=C2=A0

On Fri, Apr = 8, 2016 at 9:49 PM, Dave Page <dpage@pgadmin.org> wrote:
=


On Fri, Apr 8, 2016 at 3:39 PM, Ak= shay Joshi <akshay.joshi@enterprisedb.com> wrote= :
Hi Dave

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


On Fri, Apr 8, 2016 at 7:43 AM, Aksh= ay Joshi <akshay.joshi@enterprisedb.com> wrote:<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;pa= dding-left:1ex">
=

- The Query Tool me= nu icon should be a glyphicon, to match the others.
=
=C2=A0
=C2=A0 =C2=A0There is no gl= yphicon available which match the Query Tool icon. I have found one = like below which is "database-search" or can you please suggest s= ome other icon.=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A03D"Inline=C2=A0

=
That one looks perfect.

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0We = can't use this icon because it's not come with Bootstrap , I have p= icked 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 Boo= tstrap's Glyphicons. I really hadn't realised how much larger the F= A 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 discu= ssed, e.g. fa-bolt?

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

OK.
=C2=A0
=

  • The View Data menu option should be on the Object= menu, which should mirror the Context menu, except options should be disab= led when not applicable instead of hidden. Note: - With current impl= ementation "Object" menu is re-created dynamically depending on t= he node clicked, so we will have to re-create the "View Data" men= u as well and it require change in the generic code. For the time being &qu= ot;View Data" menu is visible in "Object" menu when appropri= ate node is selected. =C2=A0 =C2=A0 =C2=A0=C2=A0
  • Please merge the functionality of the Refresh a= nd 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 sc= reenshot (Modified-Data-Grid).=C2=A0
  • Please adjust the height of the Delete icon in the Edit G= rid, such that it doesn't force the row height to be higher than it sho= uld 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 hav= e added one more=C2=A0container=C2=A0to show the title. Please refer the at= tached screenshots (Modified-Query-Tool)=C2=A0
  • Query result= s should have spaces converted to "&nbsp;", so t= hat proper indenting is maintained (for example, on EXPLAIN queries)=
    • To fix this we have added css style &quo= t;white-space: pre-wrap;", but it changes t= he backgrid cell size. Please refer the screenshot=C2=A0(Modified-Data-Grid).=C2=A0
Hmmm. I'd rath= er 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 g= ets focus or something?
=C2=A0
=C2=A0 =C2=A0Please review the sc= reenshots and please let me know will it looks good.

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

--
Dave = Page
Blog: htt= p://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: <= a href=3D"http://www.enterprisedb.com" target=3D"_blank">http://www.enterpr= isedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Principal Software Engineer=C2= =A0


=
Pho= ne: +91 20-3058-9517
Mobile: +91 976-788-8246<= /div>



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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Compan= y
--001a114ab7b0d8f824053048f8ac-- --001a114ab7b0d8f828053048f8ad 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== --001a114ab7b0d8f828053048f8ad--