Mobile: fix side-menu width on tablet devices

[Tolu-Mals]

Summary

This pull request fixes the issue of the side-menu becoming too wide on devices with wider screens like tablet devices. It's a part of the Tablet Layout Project

[laurent22]

Thanks Tolu. You have some conflicts that need to be resolved

[Tolu-Mals]

I've resolved the conflicts.

[laurent22]

There are CI errors. You need to run yarn install and commit again

[Tolu-Mals]

I'm not sure how to go about this. After running yarn install, do I need to stage any changes or I'm fine with just running git commit --allow-empty --allow-empty-message and pushing?

[laurent22]

Please resolve the conflicts.

[laurent22]

Thanks for the review @personalizedrefrigerator! @Tolu-Mals, nearly there - just these small issues I mentioned and we can merge.

[personalizedrefrigerator]

It looks like the mobile tests aren't being found:

➤YN0000: [@joplin/app-mobile]: Process started
➤YN0000: [@joplin/app-mobile]: No tests found, exiting with code 1
➤YN0000: [@joplin/app-mobile]: Run with `--passWithNoTests` to exit with code 0
➤YN0000: [@joplin/app-mobile]: In /home/runner/work/joplin/joplin/packages/app-mobile
➤YN0000: [@joplin/app-mobile]:   175 files checked.
➤YN0000: [@joplin/app-mobile]:   testMatch: **/*.test.js - 0 matches
➤YN0000: [@joplin/app-mobile]:   testPathIgnorePatterns: /home/runner/work/joplin/joplin/packages/app-mobile/node_modules/ - 175 matches
➤YN0000: [@joplin/app-mobile]:   testRegex:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Pattern:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Process exited (exit code 1), completed in 4s 175ms

https://github.com/laurent22/joplin/runs/7471464033?check_suite_focus=true#step:10:4272

You might need to update tsconfig.json — my earlier PR configured things to use .test.ts files directly (instead of compiling them into .test.js files).

[Tolu-Mals]

It looks like the mobile tests aren't being found:

➤YN0000: [@joplin/app-mobile]: Process started
➤YN0000: [@joplin/app-mobile]: No tests found, exiting with code 1
➤YN0000: [@joplin/app-mobile]: Run with `--passWithNoTests` to exit with code 0
➤YN0000: [@joplin/app-mobile]: In /home/runner/work/joplin/joplin/packages/app-mobile
➤YN0000: [@joplin/app-mobile]:   175 files checked.
➤YN0000: [@joplin/app-mobile]:   testMatch: **/*.test.js - 0 matches
➤YN0000: [@joplin/app-mobile]:   testPathIgnorePatterns: /home/runner/work/joplin/joplin/packages/app-mobile/node_modules/ - 175 matches
➤YN0000: [@joplin/app-mobile]:   testRegex:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Pattern:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Process exited (exit code 1), completed in 4s 175ms

https://github.com/laurent22/joplin/runs/7471464033?check_suite_focus=true#step:10:4272

You might need to update tsconfig.json — my earlier PR configured things to use .test.ts files directly (instead of compiling them into .test.js files).

Thanks. I used my own tsconfig while resolving the merge conflicts. I'm working on it though.

[Tolu-Mals]

It looks like the mobile tests aren't being found:

➤YN0000: [@joplin/app-mobile]: Process started
➤YN0000: [@joplin/app-mobile]: No tests found, exiting with code 1
➤YN0000: [@joplin/app-mobile]: Run with `--passWithNoTests` to exit with code 0
➤YN0000: [@joplin/app-mobile]: In /home/runner/work/joplin/joplin/packages/app-mobile
➤YN0000: [@joplin/app-mobile]:   175 files checked.
➤YN0000: [@joplin/app-mobile]:   testMatch: **/*.test.js - 0 matches
➤YN0000: [@joplin/app-mobile]:   testPathIgnorePatterns: /home/runner/work/joplin/joplin/packages/app-mobile/node_modules/ - 175 matches
➤YN0000: [@joplin/app-mobile]:   testRegex:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Pattern:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Process exited (exit code 1), completed in 4s 175ms

https://github.com/laurent22/joplin/runs/7471464033?check_suite_focus=true#step:10:4272

You might need to update tsconfig.json — my earlier PR configured things to use .test.ts files directly (instead of compiling them into .test.js files).

The typescript files for the tests aren't being compiled when I run 'yarn run tsc' locally.

[Tolu-Mals]

It looks like the mobile tests aren't being found:

➤YN0000: [@joplin/app-mobile]: Process started
➤YN0000: [@joplin/app-mobile]: No tests found, exiting with code 1
➤YN0000: [@joplin/app-mobile]: Run with `--passWithNoTests` to exit with code 0
➤YN0000: [@joplin/app-mobile]: In /home/runner/work/joplin/joplin/packages/app-mobile
➤YN0000: [@joplin/app-mobile]:   175 files checked.
➤YN0000: [@joplin/app-mobile]:   testMatch: **/*.test.js - 0 matches
➤YN0000: [@joplin/app-mobile]:   testPathIgnorePatterns: /home/runner/work/joplin/joplin/packages/app-mobile/node_modules/ - 175 matches
➤YN0000: [@joplin/app-mobile]:   testRegex:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Pattern:  - 0 matches
➤YN0000: [@joplin/app-mobile]: Process exited (exit code 1), completed in 4s 175ms

https://github.com/laurent22/joplin/runs/7471464033?check_suite_focus=true#step:10:4272

You might need to update tsconfig.json — my earlier PR configured things to use .test.ts files directly (instead of compiling them into .test.js files).

Oh it's exactly as you said. @personalizedrefrigerator . I read 'jest config' when you mentioned 'tsconfig.json' earlier. The current tsconfig.json is ignoring typescript test files. Fixing that now.

[personalizedrefrigerator]

It looks like a dependency is missing! Try running yarn install again and committing changes to yarn.lock.

Error: ➤YN0000: │ root@workspace:.STDERR➤YN0000: [@joplin/app-mobile]: tools/buildInjectedJs.ts(6,45): error TS7016: Could not find a declaration file for module 'fs-extra'. 'D:/a/joplin/joplin/packages/app-mobile/node_modules/fs-extra/lib/index.js' implicitly has an 'any' type.

(A dependency on @types/fs-extra was added in a recently-merged PR)

[Tolu-Mals]

It looks like a dependency is missing! Try running yarn install again and committing changes to yarn.lock.

Error: ➤YN0000: │ root@workspace:.STDERR➤YN0000: [@joplin/app-mobile]: tools/buildInjectedJs.ts(6,45): error TS7016: Could not find a declaration file for module 'fs-extra'. 'D:/a/joplin/joplin/packages/app-mobile/node_modules/fs-extra/lib/index.js' implicitly has an 'any' type.

(A dependency on @types/fs-extra was added in a recently-merged PR)

Thanks. I'll try that.

[Tolu-Mals]

It looks like a dependency is missing! Try running yarn install again and committing changes to yarn.lock.

Error: ➤YN0000: │ root@workspace:.STDERR➤YN0000: [@joplin/app-mobile]: tools/buildInjectedJs.ts(6,45): error TS7016: Could not find a declaration file for module 'fs-extra'. 'D:/a/joplin/joplin/packages/app-mobile/node_modules/fs-extra/lib/index.js' implicitly has an 'any' type.

(A dependency on @types/fs-extra was added in a recently-merged PR)

After running yarn install there were no changes in yarn.lock

[Tolu-Mals]

A test is now failing due to a linter error in getResponsiveValue.ts:

  41:10  error  Expected '!==' and instead saw '!='  eqeqeq
  45:9   error  Expected '!==' and instead saw '!='  eqeqeq
  49:9   error  Expected '!==' and instead saw '!='  eqeqeq
  57:9   error  Expected '!==' and instead saw '!='  eqeqeq
  65:25  error  Expected '!==' and instead saw '!='  eqeqeq
  69:26  error  Expected '!==' and instead saw '!='  eqeqeq
  73:27  error  Expected '!==' and instead saw '!='  eqeqeq

eslint expects only strict equality to be used. I'll see if there's a way to rewrite the logic without the '!='

[Tolu-Mals]

I think this is ready to be merged.

[laurent22]

I think it's good now but please resolve the yarn.lock conflict

[laurent22]

Tests are failing: https://github.com/laurent22/joplin/runs/7729267796?check_suite_focus=true#step:10:4324

[Tolu-Mals]

Tests are failing: https://github.com/laurent22/joplin/runs/7729267796?check_suite_focus=true#step:10:4324

Thank you. I just made a fix, I think they should pass now.

[Tolu-Mals]

@laurent22 It's good now.

[laurent22]

Unfortunately still a yarn.lock conflict. A new one this time.

[laurent22]

Ok I've tested it on simulator and it looks good, thanks Tolu!