Thread: Improve UX of YUM/DNF download form

> On 8 Jun 2020, at 13:53, Dave Page <dpage@pgadmin.org> wrote:

> The attached patch changes that to create a single commented script which can be copied, pasted and run in one go speeding up and simplifying the process for the user.

Good idea, this is IMO an improvement for the users.

+  <li>Copy, paste and run the setup script:
Should the wording here perhaps reflect that there are optional steps, along
the lines of "copy, paste and run the relevent parts of the setup script".

+      // TODO: Remove
Any reason to keep these instead of removing as part of this?

On 6/8/20 10:55 AM, Dave Page wrote:
> Here's an updated patch with the changes suggested below, as well as
> similar simplification for the Debian/Ubuntu pages.

Overall LGTM. Given it's in a "<pre>" block, would we be able to use
newlines (\n) instead of "<br />"? I'd like to avoid introducing more
<br /> if possible, but if that's the way it goes, so be it.

Certainly a nice improvement. Thanks!

On 6/8/20 11:23 AM, Dave Page wrote:
>
>
> On Mon, Jun 8, 2020 at 4:14 PM Jonathan S. Katz <jkatz@postgresql.org
> <mailto:jkatz@postgresql.org>> wrote:
>
>     On 6/8/20 10:55 AM, Dave Page wrote:
>     > Here's an updated patch with the changes suggested below, as well as
>     > similar simplification for the Debian/Ubuntu pages.
>
>     Overall LGTM. Given it's in a "<pre>" block, would we be able to use
>     newlines (\n) instead of "<br />"? I'd like to avoid introducing more
>     <br /> if possible, but if that's the way it goes, so be it.
>
>
> Yes, yes we can. Fixed in my tree, along with the pre-existing ones.
>  
>
>
>     Certainly a nice improvement. Thanks!
>
>
> Yw :-)

Awesome. If said fixes do work, then I'm a +1 for adding it.

On Mon, Jun 8, 2020 at 5:24 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:

On 6/8/20 11:23 AM, Dave Page wrote:
>
>
> On Mon, Jun 8, 2020 at 4:14 PM Jonathan S. Katz <jkatz@postgresql.org
> <mailto:jkatz@postgresql.org>> wrote:
>
>     On 6/8/20 10:55 AM, Dave Page wrote:
>     > Here's an updated patch with the changes suggested below, as well as
>     > similar simplification for the Debian/Ubuntu pages.
>
>     Overall LGTM. Given it's in a "<pre>" block, would we be able to use
>     newlines (\n) instead of "<br />"? I'd like to avoid introducing more
>     <br /> if possible, but if that's the way it goes, so be it.
>
>
> Yes, yes we can. Fixed in my tree, along with the pre-existing ones.
>  
>
>
>     Certainly a nice improvement. Thanks!
>
>
> Yw :-)

Awesome. If said fixes do work, then I'm a +1 for adding it.

+1 in general. The one thing about the implementation that irks me a bit is that you picked id=script, because when I read it my mind just automatically thinks <script>. But that's not a big thing :)

If you want to make it even more fancy, add a button that copies it to the clipboard and does so without including the comments?

I wonder if it would make sense to still keep the client and server setups in separate boxes, so you can easily just select "all for client"? That is, one step that does reporpm + client, and a separate step that has all the stuff for server?

Or if we think that's too complicated, then I suggest we just zap the part about the server being optional and merge it all into a single set, where you dnf/yum install both server and client at the same time. Having a comment in a copy/paste script that says "the rest is optional" just feels weird when the whole point of the exercise is to make it more easy to copy/paste.

I had a note for myself on the debian side as well, to make it more copy/pastetable :) Thanks for taking care of that one!

Here's another updated patch:

- Don't use an id of "script"

- Add a button to the top-right of the script area to copy the contents to the clipboard. Once clicked, the label changes for 3 seconds to indicate to the user that the action occurred.

If folks are happy with this, once a consensus is made on the issue of whether to include the client install separately, I'll update if needed and push.

On Tue, Jun 9, 2020 at 9:37 AM Dave Page <dpage@pgadmin.org> wrote:

On Mon, Jun 8, 2020 at 4:57 PM Magnus Hagander <magnus@hagander.net> wrote:

On Mon, Jun 8, 2020 at 5:24 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:

On 6/8/20 11:23 AM, Dave Page wrote:
>
>
> On Mon, Jun 8, 2020 at 4:14 PM Jonathan S. Katz <jkatz@postgresql.org
> <mailto:jkatz@postgresql.org>> wrote:
>
>     On 6/8/20 10:55 AM, Dave Page wrote:
>     > Here's an updated patch with the changes suggested below, as well as
>     > similar simplification for the Debian/Ubuntu pages.
>
>     Overall LGTM. Given it's in a "<pre>" block, would we be able to use
>     newlines (\n) instead of "<br />"? I'd like to avoid introducing more
>     <br /> if possible, but if that's the way it goes, so be it.
>
>
> Yes, yes we can. Fixed in my tree, along with the pre-existing ones.
>  
>
>
>     Certainly a nice improvement. Thanks!
>
>
> Yw :-)

Awesome. If said fixes do work, then I'm a +1 for adding it.

+1 in general. The one thing about the implementation that irks me a bit is that you picked id=script, because when I read it my mind just automatically thinks <script>. But that's not a big thing :)

I can change that.

 

If you want to make it even more fancy, add a button that copies it to the clipboard and does so without including the comments?

Yeah, I mentioned that in my first message as a future improvement. Baby steps.

 

I wonder if it would make sense to still keep the client and server setups in separate boxes, so you can easily just select "all for client"? That is, one step that does reporpm + client, and a separate step that has all the stuff for server?

Or if we think that's too complicated, then I suggest we just zap the part about the server being optional and merge it all into a single set, where you dnf/yum install both server and client at the same time. Having a comment in a copy/paste script that says "the rest is optional" just feels weird when the whole point of the exercise is to make it more easy to copy/paste.

Yeah, that crossed my mind as well. The primary audience for this is the first time user, and they're likely going to want to install the server, so my inclination is to remove (or maybe comment out?) the line to install the client only.

What do others think, keeping in mind that the aim here is to make it as easy as possible for the first time user (once they're hooked...), and not to come up with a covers-all-possible scenarios solution?

 

I had a note for myself on the debian side as well, to make it more copy/pastetable :) Thanks for taking care of that one!

On Tue, Jun 9, 2020 at 12:11 PM Dave Page <dpage@pgadmin.org> wrote:

Here's another updated patch:

- Don't use an id of "script"

- Add a button to the top-right of the script area to copy the contents to the clipboard. Once clicked, the label changes for 3 seconds to indicate to the user that the action occurred.

If folks are happy with this, once a consensus is made on the issue of whether to include the client install separately, I'll update if needed and push.

Sweet, I like it. The one thing I'd like to see improved is that the script could be without the comments, making it more clear for the "pasted into terminal" results.

I did notice a pre-existing problem in the Ubuntu/Debian downloads -- it doesn't actually contain the step of "apt-get install" when one uses the pg apt repo. It probably should, if nothing else then for consistency with the RedHat ones. (And they should also do the same wrt installing the client)

//Magnus

 

On Tue, Jun 9, 2020 at 9:37 AM Dave Page <dpage@pgadmin.org> wrote:

On Mon, Jun 8, 2020 at 4:57 PM Magnus Hagander <magnus@hagander.net> wrote:

On Mon, Jun 8, 2020 at 5:24 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:

On 6/8/20 11:23 AM, Dave Page wrote:
>
>
> On Mon, Jun 8, 2020 at 4:14 PM Jonathan S. Katz <jkatz@postgresql.org
> <mailto:jkatz@postgresql.org>> wrote:
>
>     On 6/8/20 10:55 AM, Dave Page wrote:
>     > Here's an updated patch with the changes suggested below, as well as
>     > similar simplification for the Debian/Ubuntu pages.
>
>     Overall LGTM. Given it's in a "<pre>" block, would we be able to use
>     newlines (\n) instead of "<br />"? I'd like to avoid introducing more
>     <br /> if possible, but if that's the way it goes, so be it.
>
>
> Yes, yes we can. Fixed in my tree, along with the pre-existing ones.
>  
>
>
>     Certainly a nice improvement. Thanks!
>
>
> Yw :-)

Awesome. If said fixes do work, then I'm a +1 for adding it.

+1 in general. The one thing about the implementation that irks me a bit is that you picked id=script, because when I read it my mind just automatically thinks <script>. But that's not a big thing :)

I can change that.

 

If you want to make it even more fancy, add a button that copies it to the clipboard and does so without including the comments?

Yeah, I mentioned that in my first message as a future improvement. Baby steps.

 

I wonder if it would make sense to still keep the client and server setups in separate boxes, so you can easily just select "all for client"? That is, one step that does reporpm + client, and a separate step that has all the stuff for server?

Or if we think that's too complicated, then I suggest we just zap the part about the server being optional and merge it all into a single set, where you dnf/yum install both server and client at the same time. Having a comment in a copy/paste script that says "the rest is optional" just feels weird when the whole point of the exercise is to make it more easy to copy/paste.

Yeah, that crossed my mind as well. The primary audience for this is the first time user, and they're likely going to want to install the server, so my inclination is to remove (or maybe comment out?) the line to install the client only.

What do others think, keeping in mind that the aim here is to make it as easy as possible for the first time user (once they're hooked...), and not to come up with a covers-all-possible scenarios solution?

 

I had a note for myself on the debian side as well, to make it more copy/pastetable :) Thanks for taking care of that one!

On Tue, Jun 9, 2020 at 11:25 AM Magnus Hagander <magnus@hagander.net> wrote:

On Tue, Jun 9, 2020 at 12:11 PM Dave Page <dpage@pgadmin.org> wrote:

Here's another updated patch:

- Don't use an id of "script"

- Add a button to the top-right of the script area to copy the contents to the clipboard. Once clicked, the label changes for 3 seconds to indicate to the user that the action occurred.

If folks are happy with this, once a consensus is made on the issue of whether to include the client install separately, I'll update if needed and push.

Sweet, I like it. The one thing I'd like to see improved is that the script could be without the comments, making it more clear for the "pasted into terminal" results.

Yeah, that might need some minor futzing to deal with, as copying to the clipboard is handled by selection ranges. Might have to copy the code into a dummy hidden div, remove the comments, and copy it from there.

I did notice a pre-existing problem in the Ubuntu/Debian downloads -- it doesn't actually contain the step of "apt-get install" when one uses the pg apt repo. It probably should, if nothing else then for consistency with the RedHat ones. (And they should also do the same wrt installing the client)

Yes, I just spotted that. It's actually a little further down. I'll see about cleaning that up.

I took a peek at the updated yum.js. There's a couple unused variables in the script generation and it doesn't escape the generated text before assigning it to the DOM node. Not an issue now as there's nothing that'd break it, but if it's ever updated to include a redirect ("<") or something else hokey it'd break. 

How about the attached?  It splits the script generation into its own function returning a string and has the archChanged() only handle updating the DOM. It uses jQuery .text(...) for the DOM update so that the contents are escaped.

I don't have the full site running locally but adding the new DOM node and copy / pasting in the browser to manipulate the live site with this code seems to work fine.

Hi

On Tue, Jun 9, 2020 at 12:20 PM Sehrope Sarkuni <sehrope@jackdb.com> wrote:

I took a peek at the updated yum.js. There's a couple unused variables in the script generation and it doesn't escape the generated text before assigning it to the DOM node. Not an issue now as there's nothing that'd break it, but if it's ever updated to include a redirect ("<") or something else hokey it'd break. 

How about the attached?  It splits the script generation into its own function returning a string and has the archChanged() only handle updating the DOM. It uses jQuery .text(...) for the DOM update so that the contents are escaped.

I don't have the full site running locally but adding the new DOM node and copy / pasting in the browser to manipulate the live site with this code seems to work fine.

Thanks. The code has changed massively since the last patch (thanks to Magnus harassing me about more changes on IM). New patch to follow - I'll look to incorporate your tweaks.

 

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 6/9/20 7:58 AM, Dave Page wrote:
> So here's the updated patch following some discussion with Magnus:
>
> - Make the copyScript function more generic so it can be used elsewhere.
> - Don't include comments and blank lines in the copied text.
> - Move the css into main.css.
> - Add the actual install step to the Debian/Ubuntu instructions.
> - Improve naming of vars, classes, ids etc.
> - Ensure the copyScript function works fine with < > and so-on in the
> script.

That certainly changed a lot overnight ;)

LGTM. I don't have any additional comments on it, so I will give a +1.