-
-
Notifications
You must be signed in to change notification settings - Fork 82
Mobile view and wording #151
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
Mobile view and wording #151
Conversation
@@ -38,7 +38,7 @@ export default function JobMutator() { | |||
const dispatch = useDispatch(); | |||
|
|||
const isSavingEnabled = () => { | |||
return notificationAdapterData.length > 0 && providerData.length > 0 && name != null && name.length > 0; | |||
return notificationAdapterData.length && providerData.length && name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not exactly the same check than I had. in your case, setting name to ' ' would resolve truthy...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it leads to a different result. I didn't notice that, it should be wrapped in a Boolean
constructor.
The thing is,
console.log(Boolean('')) // false
Assuming notificationAdapter.length=1
and providerData.length=1
and name=''
:
console.log(1 && 1 && ''); // ''
It works when wrapped though:
console.log(Boolean(1 && 1 && '')); // false
console.log(Boolean(1 && 1 && 'a')); // true
console.log(Boolean(1 && 1 && 'anythingelse')); // true
@@ -27,20 +27,24 @@ export default function Login() { | |||
}, []); | |||
|
|||
const tryLogin = async () => { | |||
if (username.length === 0 || password.length === 0) { | |||
setError('Username and password are mandatory.'); | |||
if (!username?.trim() || !password) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh, I thought about using a toast here as well, but found that showing the in the login form directly looks nicer, therefor I think using a toast here is not ideal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You meant just here, right? I changed it back, let me know if that's better :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
This MR aims to:
Style changes preview:
As an example an iPhone 12 was used as mobile.