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

DRAFT: feat - Storage and Uploads (for Feedback) #11696

Draft
wants to merge 55 commits into
base: main
Choose a base branch
from
Draft

Conversation

dthyresson
Copy link
Contributor

This PR combines @Josh-Walker-GM storage rework branch a my uploads rework.

Making a draft PR so that team can add comments and feedback.

Feature may com in under separate PRs when functionally complete.

@dthyresson dthyresson self-assigned this Oct 15, 2024
@dthyresson dthyresson added the changesets-ok Override the changesets check label Oct 15, 2024
<section>
<div {...getRootProps({ className: combinedClassName })}>
<input {...getInputProps({ name })} />
<p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that I look at this, it might be better to have this render some children instead of setting messages. and then maybe can place preview renderers where one wants to and well as define message components. Would need to export out some of the isDragActive and accpectedfiles to use perhaps.


constructor({ secret }: { secret: string }) {
super()
this.secret = secret
Copy link
Member

Choose a reason for hiding this comment

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

I think we should throw an error here if secret is undefined or some other falsy value. The types are supposed to protect us from this, but often the secret comes from an env var and the types for those are unreliable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc' @Josh-Walker-GM on this ^

Comment on lines 32 to 33
rejectClassName = '',
buttonClassName = '',
Copy link
Member

Choose a reason for hiding this comment

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

Might want to update .vscode/settings.json to add these to the list of Tailwind class attributes if the user has TW setup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will if we decide to keep the button as an option. I wonder if better not just to expose the open and let people render buttons and messages as children of the drops zone instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changesets-ok Override the changesets check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants