Scroll to the error message by default #1023
Conversation
|
Can you make a gif comparing both results? |
|
Current state - you won't recognise any errors that occur In the previous commit we added a parameter |
src/core/index.js
Outdated
| @@ -183,7 +186,8 @@ export const ui = { | |||
| popupOptions: lock => getUIAttribute(lock, 'popupOptions'), | |||
| primaryColor: lock => getUIAttribute(lock, 'primaryColor'), | |||
| authButtonsTheme: lock => getUIAttribute(lock, 'authButtonsTheme'), | |||
| rememberLastLogin: m => tget(m, 'rememberLastLogin', getUIAttribute(m, 'rememberLastLogin')) | |||
| rememberLastLogin: m => tget(m, 'rememberLastLogin', getUIAttribute(m, 'rememberLastLogin')), | |||
| scrollGlobalMessagesIntoView: lock => getUIAttribute(lock, 'scrollGlobalMessagesIntoView') | |||
There was a problem hiding this comment.
shouldn't this be tget(m, 'scrollGlobalMessagesIntoView', getUIAttribute(m, 'scrollGlobalMessagesIntoView')),?
There was a problem hiding this comment.
what's the difference? actually, I am not really sure what tget,tgetUI, etc are doing. However, all properties above don't use tget, so I would assume the current way to retrieve the option is sufficient.
There was a problem hiding this comment.
Yup. you're right. I was just looking at the rememberLastLogin option, but it is a special case.
src/core/index.js
Outdated
| @@ -211,7 +215,8 @@ function extractAuthOptions(options) { | |||
| sso, | |||
| state, | |||
| nonce | |||
| } = options.auth || {}; | |||
| } = | |||
There was a problem hiding this comment.
no need to break line here
There was a problem hiding this comment.
Seems like this was changed by prettier
|
That's amazing. Thanks for the PR 🎉 |
|
Alright, we're good. I just requested some feedback from our ui/ux designer. I'll wait for him before merging this. Thaaaaaaanks for the PR 👍 |
|
Added two small commits to fix one of the new tests and add some documentation for the new parameter to the README |
|
Hey, can you rebase this? THanks! |
m-idler
force-pushed
the
scroll-global-lock-message-into-view
branch
from
e1469fa
to
9dae1ec
Compare
|
I just tested and what I saw is the scroll it varies depends of the height. This happen too with you?
|
|
I think you have to refresh after resizing the height, no? |
|
@luisrudge with refresh. Same:
|
|
thanks @beneliflo. Can you take a look @m-idler? |
|
I can reproduce the mentioned behaviour with small viewport heights too. After playing around with it a little bit, it seems that it's not caused by the scrolling itself but by the focus being set to the first input later. if (!prevProps.error && error) {
const input = this.findAutofocusInput();
if (input) {
// TODO clear timeout
setTimeout(() => input.focus(), 1000);
}
return;
}For chrome, it seems that the focused input is vertically-center-scrolled into the viewport, in firefox it's just scrolled at any position into the viewport. You can see the behaviour of Chrome and Firefox in the attached gifs. Im not sure if it's a (good) solution, to skip setting the focus when
|
|
Thanks @m-idler. Just to give you a feedback, I'm discussing internally with our design team if we should keep the scroll-to-input behaviour at all. I'm also discussing if we should make |
luisrudge
force-pushed
the
scroll-global-lock-message-into-view
branch
from
9dae1ec
to
e3e7e15
Compare
…MessagesIntoView to `true`
luisrudge
force-pushed
the
scroll-global-lock-message-into-view
branch
from
e3e7e15
to
22cc74d
Compare
|
Thanks @m-idler for all your effort. I made a small change to make this the default behaviour and removing the scroll-to-input behaviour-on-error that you pointed out. Starting from the next release, |
|
🎉 |
🚀
luisrudge
changed the title
luisrudge
changed the title
Messages might appear outside of users viewport. This PR fixes this by scrolling global messages into the users viewport. Hiding the users keyboard doesn't fix the problem because of the huge amount of different resolutions and display ratios.
see #959