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

Autosubmit token by paste and a bit more #583

Merged

Conversation

akellbl4
Copy link
Collaborator

@akellbl4 akellbl4 commented Jan 26, 2020

@umputun
Copy link
Owner

umputun commented Jan 26, 2020

ERROR in /srv/frontend/app/components/comment/comment.test.tsx
337
[tsl] ERROR in /srv/frontend/app/components/comment/comment.test.tsx(280,26)
338
      TS2344: Type '{ user: User | null; data: Comment; repliesCount?: number | undefined; post_info: PostInfo | null; isUserBanned?: boolean | undefined; isCommentsDisabled: boolean; editMode?: CommentMode | undefined; ... 8 more ...; uploadImage?: ((image: File) => Promise<...>) | undefined; } & Partial<...> & AllHTMLAttributes<...> ...' does not satisfy the constraint 'string | number | symbol'.
339
  Type '{ user: User | null; data: Comment; repliesCount?: number | undefined; post_info: PostInfo | null; isUserBanned?: boolean | undefined; isCommentsDisabled: boolean; editMode?: CommentMode | undefined; ... 8 more ...; uploadImage?: ((image: File) => Promise<...>) | undefined; } & Partial<...> & AllHTMLAttributes<...> ...' is not assignable to type 'symbol'.
340
npm ERR! code ELIFECYCLE
341
npm ERR! errno 2
342
npm ERR! [email protected] build: `webpack --mode production && es-check es5 './public/*.js'`
343
npm ERR! Exit status 2
344
npm ERR! 
345
npm ERR! Failed at the [email protected] build script.

@akellbl4
Copy link
Collaborator Author

@umputun fixed. I just forgot to push one of the files with which I am working now.

paskal
paskal previously approved these changes Jan 26, 2020
Copy link
Collaborator

@paskal paskal left a comment

Choose a reason for hiding this comment

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

Auto-submit works fine for me in Firefox for both auth and subscription. Invalid (but correct) token sent automatically and shows validation error without pushing the submit button as well.

@umputun
Copy link
Owner

umputun commented Jan 26, 2020

I have not tried it, but not really sure I get how all of this supposed to work. So, user pastes token and it got submitted by itself? If that's the case, why the Submit button even needed? And, in addition, this automatic submission feels kind of exotic and unusual to me.

@akellbl4
Copy link
Collaborator Author

@umputun yep, if a user pastes the correct token it sends automatically.
"Send" button is needed not to confuse a user because without the button the form might look broken.

@paskal
Copy link
Collaborator

paskal commented Jan 27, 2020

There might be errors like "Too many requests, try later" and I would prefer having the submit button all the time to deal with them manually in case it's needed.

After automatic submit user end up either logged in (with auth) or with page "you are subscribed, click here to unsubscribe" (notifications) - this is the desired output of the process user started. It might be surprising to skip the "push submit button" step but the user won't feel lost in my opinion.

I have the same mechanics working for my bank 3-D Secure pin-code from SMS prompt, I recall being surprised in a good way when I found I don't need to push any buttons after I entered expected four digits. My vote is for introducing this feature in a way it was implemented in this PR.

@umputun
Copy link
Owner

umputun commented Jan 27, 2020

ok, convinced. As soon as we get approval from @Mavrin I'll merge

Mavrin
Mavrin previously approved these changes Jan 27, 2020
@@ -43,6 +44,7 @@
"@types/redux-mock-store": "^1.0.1",
"@typescript-eslint/eslint-plugin": "^2.5.0",
"@typescript-eslint/parser": "^2.5.0",
"atob": "^2.1.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks unnecessary or use for node

const atob = (value) => Buffer.from(value, `base64`).toString();

@umputun umputun merged commit d22e364 into umputun:master Jan 28, 2020
Smolevich added a commit to Smolevich/remark that referenced this pull request Jan 28, 2020
* Use lib for typing tests instead copied types

* add jwt utils

* Add autosending

* fix

* tests

* remove atob and fix tests
@umputun umputun added this to the v1.6 milestone Apr 12, 2020
@akellbl4 akellbl4 deleted the feature/538_autosubmit-token-by-paste branch June 13, 2021 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants