8000 Add UI for editing type definitions by georgefst · Pull Request #1352 · hackworthltd/primer · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add UI for editing type definitions #1352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Add UI for editing type definitions #1352

wants to merge 7 commits into from

Conversation

georgefst
Copy link
Contributor
@georgefst georgefst commented Jun 2, 2025 •
edited
Loading

After #1351, which this is based on top of, this is the final major feature for the new frontend. We can now build any program!

UI is again proof-of-concept, but I don't expect to make major changes there for now. In fact, I think that while it's hardly intuitive in its current form (better labels would help for a start, but this is true for much of the current UI), this is the direction we should be heading in. The choice we've made to only show one definition at a time frees us from worrying about showing the whole type definition as connected, which was always a bit weird in the React frontend. So we now get back some of the clarity of the form-based UI we had in our PureScript frontend, but with the ability to actually create arbitrarily-complex type defs (the fully form-based approach was never going to generalise properly, as I argued a lot back in the day).

Code needs some fairly minor tidying up, which will be done once #1351 is merged.

There is also at least one bug: selecting a kind node in a constructor field type causes the root node to be selected for some reason.

Some examples:
Screenshot From 2025-06-02 11-24-23
Screenshot From 2025-06-02 11-25-02
image

Note that seeing as we don't actually have buttons for adding definitions yet, I had to add this temporary code in order to get one to play around with:

diff --git a/primer-miso/src/Primer/Miso.hs b/primer-miso/src/Primer/Miso.hs
index 8ddb47a7..0327f592 100644
--- a/primer-miso/src/Primer/Miso.hs
+++ b/primer-miso/src/Primer/Miso.hs
@@ -230,7 +230,7 @@ start =
             }
       , update = updateModel
       , view = viewModel
-      , subs = []
+      , subs = pure \s -> liftIO $ s $ ApplyProgAction $ AddTypeDef (Primer.qualifyName (Primer.mkSimpleModuleName "Main") "T") $ ASTTypeDef [] [] []
       , events = defaultEvents
       , initialAction = NoOp "start"
       , mountPoint = Nothing
@@ -299,10 +299,12 @@ data Action
   | ToggleFullscreenEval
   | ChooseRedex ID
   | StepBackEval
+  | ApplyProgAction ProgAction

 updateModel :: Action -> Model -> Effect Action Model
 updateModel =
   fromTransition . \case
+    ApplyProgAction action -> runMutation (Edit [action]) >> resetActionPanel
     NoOp _ -> pure ()
     SelectDef d -> do
       #readOnlySelection .= Nothing

georgefst added 7 commits June 2, 2025 01:50
@georgefst
refactor(primer-miso): abstract node highlighting
cb9e73e 
Will allow other uses and extra constructors.

Signed-off-by: George Thomas <georgefsthomas@gmail.com>
@georgefst
refactor(primer-miso): support animating nodes
971be65 
This is unused for now.

Signed-off-by: George Thomas <georgefsthomas@gmail.com>
@georgefst
chore(primer-miso): add comment about initial eval options
4ca8713 
Signed-off-by: George Thomas <georgefsthomas@gmail.com>
@georgefst
feat(primer-miso): display fullscreen button separately from eval opt…
c1cccff 
…ions

Signed-off-by: George Thomas <georgefsthomas@gmail.com>
@georgefst
refactor(primer-miso): qualify ID for eval options element
321681a 
IDs should be unique within a page, so it's best to be careful to avoid clashes.

Signed-off-by: George Thomas <georgefsthomas@gmail.com>
@georgefst
feature(primer-miso): support interactive evaluation
9362c2e 
This is a proof-of-concept, with UI choices being driven largely by implementation simplicity. One quirk is that we can't change eval opts on the fly during interactive evaluation, because this resets the evaluation state completely, i.e. it takes the history back to a singleton list. This is because we also want full (i.e. non-interactive) eval to be in sync with the opts.

Note that we no longer show the timeout error, since we now consider timing out to be fine and normal, as interaction can be continued interactively. Besides, it's obvious whether an expression has finished eval, by whether there are redexes highlighted (at least for small expressions), and anyway, the old `"No selection for eval"` message was never actually shown, since we mistakenly didn't show the `error` field when `expr` was `Nothing`. We still show `"No definition selected for evaluation"`, so nothing has been lost with this change. For similar reasons, we set `stepLimit = 0`, instead of the arbitrary `stepLimit = 10`, which is also consistent with using `False` for checkboxes, in terms of initial state being reflected in the UI.

Signed-off-by: George Thomas <georgefsthomas@gmail.com>
@georgefst
typedefs
4f46eb6 
Signed-off-by: George Thomas <georgefsthomas@gmail.com>
@dhess
Copy link
Member
dhess commented Jun 6, 2025

The UI is obviously extremely basic, but it's probably best to start this way until we figure out what we want.

Do you think that displaying constructors top-down is the right way to go, or was that just the most expeditious implementation?

@georgefst
Copy link
Contributor Author

Do you think that displaying constructors top-down is the right way to go, or was that just the most expeditious implementation?

Well, seeing as we've got two layers of nesting (type defs have constructors, and constructors have fields), some sort of grid seemed natural. I'm not particularly attached to any specifics beyond that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@georgefst @dhess
0