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

S3Upload UI fixes #15179

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

S3Upload UI fixes #15179

wants to merge 1 commit into from

Conversation

deanhannigan
Copy link
Contributor

Description

A couple of minor fixes for the S3Upload component for issues that I came across while testing.

Addresses

  • If you moved the S3Upload component in the builder the progress circle would appear and remain visible
  • Minor change to allow binding of the S3Upload picker value within its own settings. This is specifically useful if you want to map the name of a file you've added in file picker directly to the File name setting.
    • Using the existing notation will now output the file name {{ My File Upload.Value.0.name }}
    • This will output the browser mimetype {{ My File Upload.Value.0.type }}

Screenshots

Original S3 issue. This is now prevented
Screenshot 2024-12-13 at 09 02 02

Launchcontrol

Minor fix for S3Upload component and added nested prop binding.

Copy link

qa-wolf bot commented Dec 13, 2024

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

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.

LGTM but one small change needed to allow all normal bindings to work.

@@ -78,12 +83,15 @@
}

const upload = async () => {
const processedFileKey = processStringSync(key, {
[$component.id]: fieldState,
Copy link
Member

Choose a reason for hiding this comment

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

This means the only context available is itself, but we need to add the normal context in here too don't we? Since you have access to all normal bindings when configuring this setting in the builder.

So we just need to pull in normal context as well:

const context = getContext("context")
...
const processedFileKey = processStringSync(key, {
  ...$context,
  [$component.id]: fieldState,
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants