Skip to content
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.

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

Binding V2 Updates #10732

Merged
merged 23 commits into from
May 31, 2023
Merged

Binding V2 Updates #10732

merged 23 commits into from
May 31, 2023

Conversation

deanhannigan
Copy link
Contributor

Description

General updates to the Bindings UX in the builder and automations. Bindings can now also be browsed and added via a new autocomplete menu

Note: The "Handlebars" tab in the binding drawer has been renamed the "Text" tab.

  • In the "Text" tab, typing {{ will prompt the bindings list to open. Clicking on an option will add it to the editor.
  • In the "JavaScript" tab, entering $ with autocomplete to $("")

Addresses:

  • BUDI-6984 - Bindings V2 - CodeMirror updates
  • BUDI-6982 - Bindings V2 - Binding drawer updates
  • BUDI-6983 - Bindings V2 - Warn when switching between HBS and JS

Screenshots

The Updated look for the bindings drawer.

  • The old bindings menu is now on the right, and collapsable allowing the code editor to make use of more space
  • The drawer layout has been condensed
Screenshot 2023-05-29 at 22 12 27

The collapsable menu

vid3

Text mode editing

Previously, the handlebars editor was just a text field. Now it utilises CodeMirror to provide autocomplete functionality and contextual highlighting

vid5

Mode Switch Check

Before switching tabs in the binding menu, the user is now altered to the fact that, that means switching editor modes.
You are now given the option clear the previous mode value and proceed, or to abort the switch.

vid4

JavaScript mode editing

Editing in the JavaScript mode behaves much the same as before, with the addition of the autocomplete suggestions when adding a new binding. Some visual tweaks have also been added for the darker themes in the builder.

vid2

Binding field focus

When altering the binding for a particular property, that property will remain in focus as long as the drawer remains open

vid6

@codecov-commenter
Copy link

codecov-commenter commented May 30, 2023

Codecov Report

Merging #10732 (007e048) into develop (2d54af3) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff            @@
##           develop   #10732   +/-   ##
========================================
  Coverage    60.15%   60.15%           
========================================
  Files          245      245           
  Lines         7521     7521           
  Branches      1405     1405           
========================================
  Hits          4524     4524           
  Misses        2757     2757           
  Partials       240      240           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@aptkingston
Copy link
Member

Looks awesome Dean!

From pulling it down and playing around, just a few things I noticed which should be easy fixes:

  • Swapping from text to JS is fine, but when swapping back from JS to text it will prompt you about losing your work, even if the field it totally empty and you haven't typed anything
  • Also when swapping from JS to text, it says "keep text" and that you'll lose your "text" value, but that should probably say "Keep JavaScript" and warn about losing the "JavaScript" value
    image
  • The helper text at the bottom references the dollar sign for bindings, even when using text
    image

A few questions, which maybe aren't so easy but just UX things which I though are worth querying. Probably no showstoppers here but I think some of these are expected behaviour and others are just nice to haves:

  • There seems to be no way to manually open the binding popover at all, is that right? It opens automatically when it's supposed to, but I can't find a way to manually open it.
  • Hoping this one is fixable, but if there's a space after the {{ in text then bindings won't open and search - it only works immediately after the {{. It feels quite natural to enter a space and I think a lot of people will do that, so if that's an easy one then I think it's worth doing!
  • Is it possible to include the category name when searching? This is how the drawer on the right works - entering something like "user" would find all bindings which include that, but also return all "current user" bindings since the word "user" is in the category. That would be a really helpful feature if possible!

Copy link
Member

@aptkingston aptkingston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @deanhannigan. Love how self contained the new editor is, and how nicely it integrates with the existing bindings. Played with this manually a good bit as well and it feels great, so let's get this in!

Just run a yarn install to fix the conflict lines leftover in yarn.lock and we're good to go.

lineHeight: "1.3",
border:
"var(--spectrum-alias-border-size-thin) solid var(--spectrum-alias-border-color)",
borderRadius: "var(--border-radius-s)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure discovering the correct variables for everything here was fun!

@@ -101,7 +101,12 @@
}
// Ignore events when typing
const activeTag = document.activeElement?.tagName.toLowerCase()
if (["input", "textarea"].indexOf(activeTag) !== -1 && e.key !== "Escape") {
const inCodeEditor =
document.activeElement?.classList?.contains("cm-content")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch to account for that.

@@ -3531,6 +3609,104 @@
validate-npm-package-name "^4.0.0"
yargs-parser "20.2.4"

<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflict looks like its been committed accidentally!

@deanhannigan deanhannigan merged commit eeb3d22 into develop May 31, 2023
@github-actions github-actions bot locked and limited conversation to collaborators May 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants