Embed editor state in URL; remove session storage #299

pete-murphy · 2022-08-27T16:59:36Z

Description of the change

Relates to #118

This removes the session storage in localStorage and persists editor state in the URL using lz-string. 2d5c9bd adds a Share URL button (not actually a button, just a clickable <li>) that copies the URL to clipboard, and gives feedback that the URL has successfully been copied to the clipboard.

Example URL

For the default editor state of

module Main where

import Prelude

import Data.Foldable (fold)
import Effect (Effect)
import TryPureScript (h1, h2, p, text, list, indent, link, render, code)

main :: Effect Unit
main =
  render $ fold
    [ h1 (text "Try PureScript!")
    , p (text "Try out the examples below, or create your own!")
    , h2 (text "Examples")
    , list (map fromExample examples)
    , h2 (text "Share Your Code")
    , p (text "A PureScript file can be loaded from GitHub from a gist or a repository. To share code using a gist, simply include the gist ID in the URL as follows:")
    , indent (p (code (text "  try.purescript.org?gist=<gist id>")))
    , p (fold
        [ text "The gist should contain PureScript module named "
        , code (text "Main")
        , text " in a file named "
        , code (text "Main.purs")
        , text " containing your PureScript code."
        ])
    , p (text "To share code from a repository, include the path to the source file as follows:")
    , indent (p (code (text "  try.purescript.org?github=/<owner>/<repo>/<branch or tag>/<file>.purs")))
    , p (fold
        [ text "The file should be a PureScript module named "
        , code (text "Main")
        , text " containing your PureScript code."
        ])
    ]
  where
  fromExample { title, source } =
    link ("https://github.com/purescript/trypurescript/master/client/examples/" <> source) (text title)

  examples =
    [ { title: "Algebraic Data Types"
      , source: "ADTs.purs"
      }
    , { title: "Loops"
      , source: "Loops.purs"
      }
    , { title: "Operators"
      , source: "Operators.purs"
      }
    , { title: "Records"
      , source: "Records.purs"
      }
    , { title: "Recursion"
      , source: "Recursion.purs"
      }
    , { title: "Do Notation"
      , source: "DoNotation.purs"
      }
    , { title: "Type Classes"
      , source: "TypeClasses.purs"
      }
    , { title: "Generic Programming"
      , source: "Generic.purs"
      }
    , { title: "QuickCheck"
      , source: "QuickCheck.purs"
      }
    ]

the URL looks like http://localhost:8080/?purs=LYewJgrgNgpgBAWQIYEsB2cDuALGAnGAKEJWAAcQ8AXOABQKgjCJPMpoBEkqkA6AMRBQwSAEaw4ACgBmQsAEpWFanACi06TADGNSes07FpZTQAqeAJ60IBAMpa8KMruwBGADRxsAJk9lPVDAAHlSeUCgAzqFw6Mxo0eFoANaeBGjMeJ5a4DCKhMCoGABcRWoa2jQAqmgoVPmFcAC8hHBwaRlwACRwssItrXAA2l6uUoEhcABE5hZ0NjD2js4AhJOKA3B%2BY8E005ZwIBA0VLhwwUjksBFwojBQIJielHAOMNzwFod4B5hoq%2BsDTw%2BbYTSaqIIXMhXNb9VphSK6ApkHp4EDAcGQiTnS4wCIAuFebwg3a2bBIAhwACaXzgAGEcjCNltJONdgBBOZ2BxOGjSFASLRIDC3OD3JDMMAotFwADitQAEhBRFLgHAkHAAOYIg7fdUECgRWqUCy8OCmEBwCJkinZZhwCCGtAatWahGeQ2XWboLSMO0neBaqJwACSHBiGH9cEqACUADJq669e6YCJFRmA8NxXTIyS2%2BAsnZTVpUSy8MjzCLc5y8SgagD8gaojQAPI2YmAAHxreT4zZwHO9MCwjZDOCsqamU5tq2HYQvEDxBrWLlLGigSASNAXGCSybDpnzu0F0HIdDpkd98eTcMuvmb7e7-cZvPEqantBlmwRc8jgKF6-ZIu6DoM6nw2JyCxVjQea8HuF6tAAur2zJXualrWvAL7SKiqp6jABpGpYnjer68CRmQ3DYGOFqRhEXxaPAd7wEgiZCMmqY-sR6QwPEUg5i%2Bx67MWpblgQlarjWeD1lqJxKo0AD0zYPGg%2BAdop%2BogGpzaiHgQpaFRzw8BqWlMR2n54N%2BPbIf2UiDk%2BrTDKhpxMehs6SiK6rLpBq5wOu0DwFuwA7lM9l9gJV7vj%2BB5XvOQE1E6cBgd8XmLDyh4wLBoVIcOCH9Dg%2BBEK02FohiOJwAA3mOtSwO69HwAAvk0w6JEkUiTNgVBUGQqbyfJMnYEqvDZMA8mibiUHySWFhjeJPLyQUUT4PJPooDxVDydiUK4vJ17Nh2lp1fIr5UNVuTEK0m1XE1GzDJVJ1ULApSTGyUAajAOmoFocBcDwZrTbicG-gdNgMU9bIcKYETmd%2BT71cOnh3adT2xiAIDdYDB50SDMDI6j3XQxjAxwweiMPTjUwAPJkPg3CUDDF61djT1UzTVB0wTsPwxVVVk090baJQYD00DWN4KDUz89keBCxzF7ExmpOPRL2hfigC6EwSovi5MkuqwussjvLBKK%2BTkwcBaAByIA8Cd6tPozYum%2BbVs22rH6icLGxG32JtPaY-10lALERAD9vA47fv-bSQcRCHUMexrcDewjPNK5MMo8fgKBffQIAarpwDACBicO9rGcqY4WgG17XO%2B1MACKEDZ0ktK4FoSQl%2BH2uN83rfaEk1dEzlhBAA

Checklist:

Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0 by @)")
Linked any existing issues or proposals that this pull request should close
Updated or added relevant documentation
Added a test for the contribution (if applicable)

pete-murphy · 2022-08-27T17:06:34Z

client/src/Try/Container.purs

@@ -141,12 +139,7 @@ component = H.mkComponent
          handleAction $ Compile Nothing

    Cache text -> H.liftEffect do


Not sure if Cache still makes sense as the action name, maybe Persist? EncodeInURL?

EncodeInUrl sounds good to me.

Renamed here 6eb73be

pete-murphy · 2022-08-27T17:07:44Z

client/src/Try/Container.purs

+withSession :: Aff { sourceFile :: Maybe SourceFile, code :: String }
+withSession = do


Also maybe the withSession name no longer makes sense, now that this doesn't take a sessionId

What are your thoughts on what this could be renamed to then?

Couldn't think of anything better 😅 so I'll just leave it

pete-murphy · 2022-09-01T17:13:34Z

client/src/Try/Container.purs

+      H.modify_ (_ + 1)
+      H.liftAff $ delay (1_500.0 # Milliseconds) 
+      H.modify_ (_ - 1)


Maybe this state would be more clear if it were a showConfirmation :: Boolean instead of Int? I used Int so that repeated clicks would keep the confirmation message showing, but that's probably not necessary.

Doesn't this block the thread because you're not wrapping the delay in an H.fork? And shouldn't some debouncer be used here:

user clicks the button, which copies the content to the clipboard and starts a debouncer. If the copy doesn't fail, a "Copied to Clipboard" message is displayed. If it does fail, a "Failed to Copy" is displayed.

if user clicks again before debouncer ends, debounder restarts

if debouncer times out, then the text changes back to "Share URL"

Then, the state would be just showConfirmation :: Boolean or perhaps Maybe String where Nothing = "Share URL" and Just s = s where s is either "Failed to Copy" or "Copied to Clipboard". If you initialized this component with a Ref, you could use that to store the reference to the debouncer's ref.

Wouldn't debouncing mean something like: we delay writing to clipboard until N milliseconds have passed without another click event? I think I'd still want to write to clipboard immediately, but just keep the notification message showing for some time after. I'm new to Halogen, so there might be a way to leverage debouncing to accomplish that that I'm unaware of; maybe I'd need to setup an event emitter and then debounce some "hide notification message" event?
I've got a prototyped solution using H.fork and storing the ForkId in state here: https://try.purescript.org/?gist=5605231047155a3bf4b834daa03b510e
I'm not sure if there's a simpler way, but that will at least allow showing either a success or failure message as appropriate. I'll plan on going with that unless I find a simpler solution.

Using H.fork (and cancelling previous handleAction) here 6eb73be

Wouldn't debouncing mean something like: we delay writing to clipboard until N milliseconds have passed without another click event?

No, we'd write to the clipboard immediately. We would delay when we change from "Copied to Clipboard" back to the original text of "Share Link". If multiple clicks happen, we keep displaying "Copied to Clipboard". Once the debouncer times out because the user hasn't clicked that button for a while, we change back to "Share Link".

I think the recent changes implement that behavior, though I'm not explicitly using a debounce function.

pete-murphy · 2022-09-01T17:15:46Z

client/src/Try/Container.purs

+      H.liftAff $ makeAff \f -> do
+        runEffectFn3 copyToClipboard url (f (Right unit)) (f (Left $ Aff.error "Failed to copy to clipboard"))


This is copying the pattern I saw with how setupIFrame is called, but maybe showing a message to the user is more appropriate than throwing an Error here.

Perhaps a simple window >>= alert would be a cheap-and-easy way to notify the user? I know it's not the best UX, but do we anticipate this failing that often?

Actually, since the "Share URL" changes to 'Copied to Clipboard" when that is clicked, could it also be changed to "Failed to Copy" when a failure occurs?

Actually, since the "Share URL" changes to 'Copied to Clipboard" when that is clicked, could it also be changed to "Failed to Copy" when a failure occurs?

Yeah, I'll plan on that 👍

Showing the error message in UI here 6eb73be

do we anticipate this failing that often?

Browser support is pretty much universal https://caniuse.com/mdn-api_clipboard_writetext. I was thinking there was a possibility that some users might have disabled writing to the clipboard, but I haven't figured out how to do that in Chrome/Firefox. In Chrome there is a setting for disabling reading from clipboard, but haven't seen a way of limiting write access.

Hmm, I’d probably want to also wrap the FFI call in try/catch in the 5% chance that navigator.clipboard.writeText doesn’t exist (in non-supported browsers)

Updated here: 973b0ff

Show copy failure message to user; rename action

JordanMartinez · 2022-09-06T02:13:27Z

client/src/Try/Container.purs

+    H.modify_ _ { showCopySucceeded = Just copySucceeded }
+    forkId <- H.fork do
      H.liftAff $ delay (1_500.0 # Milliseconds) 
-      H.modify_ (_ - 1)
-  render :: Int -> H.ComponentHTML Unit () Aff
-  render n =
+      H.modify_ _ { showCopySucceeded = Nothing }
+    H.modify_ _ { forkId = Just forkId }


Rather than modifying the state twice...

copySucceed <- ... H.modify_ _ { showCopySucceeded = Just copySucceeded } ... H.modify_ _ { forkId = Just forkId }

Why not do a single state update at the end?

copySucceed <- ... forkId <- H.fork do ... H.modify_ $ const { forkId: Just forkId, showCopySucceeded: Just copySucceeded }

Updated here 1f4b17a

JordanMartinez · 2022-09-06T02:13:50Z

I'll have to try this out locally, but I this looks good to me.

JordanMartinez · 2022-09-06T15:25:36Z

I've checked this out locally and played with it for a bit. The code works, but there are a few UX things to figure out now. I think these can be solved in a separate PR.

First, if I load a gist, the URL will include the gist in the query param in addition to the purs query param for the lz-compresed code. I think that's useful because it can show you what the original gist was. However, if I modify the code after loading the gist, the URL doesn't drop the gist part. Thus, I can still click on "View > Gist" to open up the original gist. However, there's nothing indicating that the gist I'll be seeing is different from the code being run in the browser due to my previous modification. There should likely be a warning that the gist has been modified since it's original load. This should also apply to the GitHub variant as well.

Second, now that we can share URLs, some may expect the URL to automatically update to whatever options the user chooses. For example, if the user chooses "View > Output", we'll see only the output, but the URL doesn't update to reflect that. If User A sends the URL to User B with the intent of discussing just the output, User B will see the side-by-side view, not the output view. This will likely be a bit confusing to both.

What are your thoughts?

JordanMartinez · 2022-09-06T15:30:08Z

Oh, one other thing. The readme needs to be updated to drop the part about session storage.

pete-murphy · 2022-09-06T15:54:59Z

First, if I load a gist, the URL will include the gist in the query param in addition to the purs query param for the lz-compresed code. I think that's useful because it can show you what the original gist was. However, if I modify the code after loading the gist, the URL doesn't drop the gist part. Thus, I can still click on "View > Gist" to open up the original gist. However, there's nothing indicating that the gist I'll be seeing is different from the code being run in the browser due to my previous modification. There should likely be a warning that the gist has been modified since it's original load. This should also apply to the GitHub variant as well.

Hmm, yeah I could see wanting to have a warning about the link to Gist / GitHub being out of sync with the current editor state, but I don't have a clear idea of what that should look like. I also think it's a bit odd to have the link under View Mode in the first place; the other options in that menu actually switch between different view modes within TPS, so the Gist / GitHub link already feels a bit unintuitive lumped in there. I guess we could emphasize that it's a link to the original code with some extra copy (the title already says "Open the original gist ..." on hover), but I don't know that that's actually helping to clarify anything 😄 .

Second, now that we can share URLs, some may expect the URL to automatically update to whatever options the user chooses. For example, if the user chooses "View > Output", we'll see only the output, but the URL doesn't update to reflect that. If User A sends the URL to User B with the intent of discussing just the output, User B will see the side-by-side view, not the output view. This will likely be a bit confusing to both.

Agreed 👍 I think the View Mode state could be tracked in the URL too, so that opening a shared URL ends up on the same exact view.

Oh, one other thing. The readme needs to be updated to drop the part about session storage.

Good catch, can do 👍

JordanMartinez · 2022-09-08T14:40:12Z

What was the reasoning behind using purs as the query parameter rather than something else (e.g. content, code, etc.)? I'm wondering if we should use a different name for that parameter so that purs can be used for something else in the future (e.g. if multiple purs versions were supported simultaneously) without breaking links made in the past.

pete-murphy · 2022-09-08T15:00:06Z

What was the reasoning behind using purs as the query parameter rather than something else (e.g. content, code, etc.)? I'm wondering if we should use a different name for that parameter so that purs can be used for something else in the future (e.g. if multiple purs versions were supported simultaneously) without breaking links made in the past.

It was an arbitrary choice, I'd be open to change it to whatever. Rust Playground uses code=<uncompressed string>, TypeScript Playground uses #code/<compressed string>, Try Reason uses reason=<compressed string>. code seems like a reasonable choice.

JordanMartinez · 2022-09-08T15:23:32Z

Yeah, using `code` sounds good.

pete-murphy · 2022-09-08T21:19:26Z

Second, now that we can share URLs, some may expect the URL to automatically update to whatever options the user chooses. For example, if the user chooses "View > Output", we'll see only the output, but the URL doesn't update to reflect that. If User A sends the URL to User B with the intent of discussing just the output, User B will see the side-by-side view, not the output view. This will likely be a bit confusing to both.

I have a separate commit for syncing the URL with the local state for Settings, but I don't know if this is the approach we want to use: pete-murphy@d5f2914. I can include it in this PR if we want.

JordanMartinez

LGTM. The other stuff can be resolved in a separate PR.

JordanMartinez · 2022-09-08T23:00:40Z

🏓 @garyb Can I get an approval on this?

garyb

Nice 👍

lz-string is new to me, but looks super useful.

JordanMartinez · 2022-09-10T20:57:38Z

I've just redeployed TPS with this change. Thanks @ptrfrncsmrph!

Embed editor state in URL; remove session storage

11d6215

pete-murphy commented Aug 27, 2022

View reviewed changes

Add to CHANGELOG

ad9b25b

pete-murphy marked this pull request as ready for review August 27, 2022 17:32

Add "Share URL" button

2d5c9bd

pete-murphy force-pushed the ptrfrncsmrph/fix-118-embed-code-in-url branch from aca3f09 to 2d5c9bd Compare August 28, 2022 16:17

pete-murphy commented Sep 1, 2022

View reviewed changes

Review feedback

6eb73be

Show copy failure message to user; rename action

pete-murphy requested a review from JordanMartinez September 4, 2022 02:05

Wrap copyToClipboard in try/catch

973b0ff

JordanMartinez reviewed Sep 6, 2022

View reviewed changes

Use H.put instead of multiple H.modify_

1f4b17a

Update README

409795b

pete-murphy force-pushed the ptrfrncsmrph/fix-118-embed-code-in-url branch from e805b7b to 409795b Compare September 7, 2022 04:22

Use 'code' as query param

0dfc21d

JordanMartinez approved these changes Sep 8, 2022

View reviewed changes

garyb approved these changes Sep 9, 2022

View reviewed changes

JordanMartinez merged commit a958b25 into purescript:master Sep 10, 2022

thomashoneyman mentioned this pull request Sep 16, 2022

Update main example to note the URL contains code #300

Merged

1 task

pete-murphy mentioned this pull request Nov 7, 2022

Sync settings with URL pete-murphy/trypurescript#2

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Embed editor state in URL; remove session storage #299

Embed editor state in URL; remove session storage #299

pete-murphy commented Aug 27, 2022 •

edited

Loading

pete-murphy Aug 27, 2022 •

edited

Loading

JordanMartinez Sep 3, 2022

pete-murphy Sep 4, 2022

pete-murphy Aug 27, 2022 •

edited

Loading

JordanMartinez Sep 3, 2022

pete-murphy Sep 4, 2022 •

edited

Loading

pete-murphy Sep 1, 2022

JordanMartinez Sep 3, 2022

pete-murphy Sep 4, 2022

pete-murphy Sep 4, 2022

JordanMartinez Sep 4, 2022

pete-murphy Sep 4, 2022

pete-murphy Sep 1, 2022

JordanMartinez Sep 3, 2022

JordanMartinez Sep 3, 2022

pete-murphy Sep 4, 2022

pete-murphy Sep 4, 2022

pete-murphy Sep 4, 2022

pete-murphy Sep 5, 2022

pete-murphy Sep 5, 2022

JordanMartinez Sep 6, 2022

pete-murphy Sep 6, 2022

JordanMartinez commented Sep 6, 2022

JordanMartinez commented Sep 6, 2022

JordanMartinez commented Sep 6, 2022

pete-murphy commented Sep 6, 2022 •

edited

Loading

JordanMartinez commented Sep 8, 2022

pete-murphy commented Sep 8, 2022 •

edited

Loading

JordanMartinez commented Sep 8, 2022 via email

pete-murphy commented Sep 8, 2022

JordanMartinez left a comment

JordanMartinez commented Sep 8, 2022

garyb left a comment

JordanMartinez commented Sep 10, 2022

		@@ -141,12 +139,7 @@ component = H.mkComponent
		handleAction $ Compile Nothing

		Cache text -> H.liftEffect do

		withSession :: Aff { sourceFile :: Maybe SourceFile, code :: String }
		withSession = do

		H.liftAff $ makeAff \f -> do
		runEffectFn3 copyToClipboard url (f (Right unit)) (f (Left $ Aff.error "Failed to copy to clipboard"))

Embed editor state in URL; remove session storage #299

Embed editor state in URL; remove session storage #299

Conversation

pete-murphy commented Aug 27, 2022 • edited Loading

pete-murphy Aug 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pete-murphy Aug 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pete-murphy Sep 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JordanMartinez commented Sep 6, 2022

JordanMartinez commented Sep 6, 2022

JordanMartinez commented Sep 6, 2022

pete-murphy commented Sep 6, 2022 • edited Loading

JordanMartinez commented Sep 8, 2022

pete-murphy commented Sep 8, 2022 • edited Loading

JordanMartinez commented Sep 8, 2022 via email

pete-murphy commented Sep 8, 2022

JordanMartinez left a comment

Choose a reason for hiding this comment

JordanMartinez commented Sep 8, 2022

garyb left a comment

Choose a reason for hiding this comment

JordanMartinez commented Sep 10, 2022

pete-murphy commented Aug 27, 2022 •

edited

Loading

pete-murphy Aug 27, 2022 •

edited

Loading

pete-murphy Aug 27, 2022 •

edited

Loading

pete-murphy Sep 4, 2022 •

edited

Loading

pete-murphy commented Sep 6, 2022 •

edited

Loading

pete-murphy commented Sep 8, 2022 •

edited

Loading