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.

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

@uppy/provider-view: allow apps to render custom forms in AuthView #4555

Closed
wants to merge 1 commit into from
Closed

@uppy/provider-view: allow apps to render custom forms in AuthView #4555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 31 additions & 21 deletions packages/@uppy/provider-views/src/ProviderView/AuthView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function GoogleIcon () {
}

function AuthView (props) {
const { pluginName, pluginIcon, i18nArray, handleAuth } = props
const { pluginName, pluginIcon, i18nArray, handleAuth, inputs } = props
// In order to comply with Google's brand we need to create a different button
// for the Google Drive plugin
const isGoogleDrive = pluginName === 'Google Drive'
Expand All @@ -48,6 +48,7 @@ function AuthView (props) {
<br />
</span>
)

return (
<div className="uppy-Provider-auth">
<div className="uppy-Provider-authIcon">{pluginIcon()}</div>
Expand All @@ -56,26 +57,35 @@ function AuthView (props) {
pluginName: pluginNameComponent,
})}
</div>
{isGoogleDrive ? (
<button
type="button"
className="uppy-u-reset uppy-c-btn uppy-c-btn-primary uppy-Provider-authBtn uppy-Provider-btn-google"
onClick={handleAuth}
data-uppy-super-focusable
>
<GoogleIcon />
{i18nArray('signInWithGoogle')}
</button>
) : (
<button
type="button"
className="uppy-u-reset uppy-c-btn uppy-c-btn-primary uppy-Provider-authBtn"
onClick={handleAuth}
data-uppy-super-focusable
>
{i18nArray('authenticateWith', { pluginName })}
</button>
)}
<form className="uppy-Provider-authForm" onSubmit={handleAuth}>
{inputs?.map((i) => (
<div className="uppy-Provider-authInput">
<label htmlFor={`uppy-Provider-authInput-${i.id}`}>
<span>{i.label}</span>
<input id={`uppy-Provider-authInput-${i.id}`} name={i.name} type={i.type || 'text'} defaultValue={i.defaultValue} />
</label>
{i.description && (<span>{i.description}</span>)}
</div>
))}
Comment on lines +61 to +69
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 this is too tightly coupled. We probably want some sort of inversion of control here, like is done for metaFields in Dashboard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is too tightly coupled to what?

I'm not sure what you would like here and what for.
I see, it might indeed be useful to have a custom render function apart from that I would need some examples of what you imagine.

Feel free to push anything if that makes it easier

Copy link
Member

Choose a reason for hiding this comment

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

The inputs is tightly coupled to a very specific HTML structure. If someone wants to change even one small thing, it's not possible. Therefor I think it's better to keep something like this flexible with inversion of control: exposing a a function which returns preact elements.

Example from Dashboard:

uppy.use(Dashboard, {
    trigger: '#pick-files',
    metaFields: [
        { id: 'name', name: 'Name', placeholder: 'file name' },
        { id: 'license', name: 'License', placeholder: 'specify license' },
        {
            id: 'caption',
            name: 'Caption',
            placeholder: 'describe what the image is about',
        },
        {
            id: 'public',
            name: 'Public',
            render({ value, onChange, required, form }, h) {
                return h('input', {
                    type: 'checkbox',
                    required,
                    form,
                    onChange: (ev) => onChange(ev.target.checked ? 'on' : ''),
                    defaultChecked: value === 'on',
                });
            },
        },
    ],
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright, makes sense! I'll try to implement that asap :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think I should reuse RenderMetaFields for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

In my poc experiment i'm doing like this:

    this.view = new ProviderViews(this, {
      provider: this.provider,
      viewType: 'list',
      showTitles: true,
      showFilter: true,
      showBreadcrumbs: true,
      renderAuthForm: () => (
        <div className="uppy-Provider-authInput">
          <label htmlFor="uppy-Provider-publicLinkURL">
            <span>{this.i18n('publicLinkURLLabel')}</span>
            <input id="uppy-Provider-publicLinkURL" name="webdavUrl" type="text" />
          </label>
          <span>{this.i18n('publicLinkURLDescription')}</span>

          <button style={{ display: 'block' }} type="submit">Submit</button>
        </div>
      ),
    })

not sure if it's the best way but render props are very flexible

Copy link
Contributor Author

@dschmidt dschmidt Aug 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, renderMetaFields provides a standard implementation if you don't care too much but allows you to specify custom render method.

This (your implementation) would work for us obviously

{isGoogleDrive ? (
<button
type="submit"
className="uppy-u-reset uppy-c-btn uppy-c-btn-primary uppy-Provider-authBtn uppy-Provider-btn-google"
data-uppy-super-focusable
>
<GoogleIcon />
{i18nArray('signInWithGoogle')}
</button>
) : (
<button
type="submit"
className="uppy-u-reset uppy-c-btn uppy-c-btn-primary uppy-Provider-authBtn"
data-uppy-super-focusable
>
{i18nArray('authenticateWith', { pluginName })}
</button>
)}
</form>
</div>
)
}
Expand Down
1 change: 1 addition & 0 deletions packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ export default class ProviderView extends View {
handleAuth={this.handleAuth}
i18n={this.plugin.uppy.i18n}
i18nArray={this.plugin.uppy.i18nArray}
inputs={this.opts.authInputs}
/>
</CloseWrapper>
)
Expand Down
22 changes: 22 additions & 0 deletions packages/@uppy/provider-views/src/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,28 @@
color: $gray-500;
}

.uppy-Provider-authForm {
display: flex;
justify-content: center;
flex-wrap: wrap;
}

.uppy-Provider-authForm input {
display: block;
width: 100%;
margin-bottom: 5px;
}

.uppy-Provider-authForm label span {
display: inline-block;
margin-bottom: 5px;
}

.uppy-Provider-authInput {
width: 100%;
margin-bottom: 15px;
}

.uppy-Provider-empty {
color: $gray-500;
}
Expand Down
Loading