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

[Question] Comment about Support for Obsidian Mobile #17

Closed
FelipeRearden opened this issue Jul 2, 2022 · 16 comments
Closed

[Question] Comment about Support for Obsidian Mobile #17

FelipeRearden opened this issue Jul 2, 2022 · 16 comments

Comments

@FelipeRearden
Copy link

Hello @pjeby

Based on two new features of Pane Relief, I would like to share some thoughts about supporting Pane Relief on Obsidian Mobile (iPadOS)

My workflow: I use Obsidian on iPad with a mouse and keyboard

Pane Relief add two spectacular new features:

1️⃣ Integration with Customizable Page Header Buttons

2️⃣ Maximize Active Pane -> Maximize Active Pane Plugin with superpowers :)


1️⃣ Customizable Page Header Buttons (CPHB)

Since CPHB is supported on Mobile, would be amazing to be able to use the integration with Pane Relief on Mobile as well.

In my humble opinion, be able to add Pane relied commands on the Note Header, is the best way to add support for Pane Relief on Mobile.

2️⃣ Maximize Active Pane (MAP)

You have no idea how excited I was when I read the release notes on version 0.1.6

I am one of the users that struggles to use MAP with Hover Editor and I just have to say thank you for adding this amazing feature to Pane Relief.

  • I even tried to raise this issue on Github

Since MAP already has support for Mobile, would be amazing to have the same workflow (HE+MAP) on Mobile as well 🙏

✋ Manual Installation

Before write this post, I tried to do an manual installation on Obsidian Mobile to see if it work:

I changed the line isDesktopOnly on manifest.json and tried to enable the plugin on Obsidia Mobile (iPadOS)

Unfortunately, it was not possible:

772B0339-CA3A-4256-9160-8E45B92963C3

⚠️ Most of the time, when we face this warning on iOS/iPadOS is because of the fact that iOS does not support lookbehind regex

I face this issue before on Mobile before:
jglev/obsidian-carry-forward#3 (comment)


Thanks for reading this post and for the new amazing features to Pane Relief.

I hope you could consider adding support for Mobile. As I said before I think theses new feature has an amazing fit for Obsidian Mobile users 🙏

I wish you a fantastic day.

@pjeby
Copy link
Owner

pjeby commented Jul 2, 2022

So, that picture doesn't show what the problem is. Do you have a console/inspector you can use to get the actual error message? If not, it's going to be difficult to help since I have not the slightest idea what the problem is, especially since PR doesn't use any regexes to my recollection. My guess is that it'll probably be the part where PR replaces window.history with its own special object, as that's the least-likely-to-be-compatible-with-Safari thing PR does.

One way to check this would be to edit main.js, look for the line that reads:

     installHistory(this);

And change it to

    //  installHistory(this);

If the plugin loads then, we'll know it's the history thing. If it is, there's another approach to history installing that might work, but if iOS is locked down to prevent the way I'm doing it, the other way I'm thinking of might also be locked down. So can't guarantee anything here. But potentially I could write the code to check for mobile, trap errors and display a message saying per-pane history not available on this platform or something, and the rest could work then.

@FelipeRearden
Copy link
Author

FelipeRearden commented Jul 2, 2022

Thanks for your reply @pjeby

So, that picture doesn't show what the problem is. Do you have a console/inspector you can use to get the actual error message?

Unfortunately, I don't have a way to open the console on Mobile :(

If not, it's going to be difficult to help since I have not the slightest idea what the problem is, especially since PR doesn't use any regexes to my recollection.

Regex was my only guess from experience with another plugins on Mobile.

My guess is that it'll probably be the part where PR replaces window.history with its own special object, as that's the least-likely-to-be-compatible-with-Safari thing PR does.
One way to check this would be to edit main.js, look for the line that reads:

     installHistory(this);

And change it to

    //  installHistory(this);

I try to do exactly what you said (see below). I only changed the line 849.

🙏 Let me know if I did exactly what is supposed to do

image

🆘 Unfortunately, after the update on main.js I was not able to enable PR the same way as before.

But potentially I could write the code to check for mobile, trap errors and display a message saying per-pane history not available on this platform or something, and the rest could work then.

Could be a great way to start the journey of PR on Mobile!

Maybe after this start, more people start to use PR on Mobile and we can get more information from Obsidian Plugin developers :)

@pjeby
Copy link
Owner

pjeby commented Jul 2, 2022

That looks like the right change, which probably means the history is not the issue.

@FelipeRearden
Copy link
Author

That looks like the right change, which probably means the history is not the issue.

I am glad to know that I did right what you asked me :)

@pjeby
Copy link
Owner

pjeby commented Jul 3, 2022

If you can find somebody with iOS development tools/skills for this, I can work with them to try to resolve it, but it would need information I presently do not have. It's possible the problem is that iOS/Safari does not have the needed Javascript language features or APIs to match the desktop, as they are notorious for lagging behind other platforms. But without knowing what specifically is breaking there isn't much else I can do.

@FelipeRearden
Copy link
Author

OK. 100% understood.

@FelipeRearden
Copy link
Author

FelipeRearden commented Jul 9, 2022

Hello @pjeby

I have some news about this FR to share with you.

Do you have a console/inspector you can use to get the actual error message?

I did some research on Discord and I found that we have a plugin that brings some console features to Mobile

https://github.com/shabegom/obsidian-mobile-logging

this way I was able to view some information from console about PR

After I try to enable PR , this is the information that we get from the console (last two lines)

C2B6B5F3-89C6-49D2-8DE8-112439963674

ℹ️ We have this information from console WITHOUT changing the line 849

ℹ️ plug-in version 0.1.8

I hope this new information could bring some light to this issue 🙏🙏🙏

@pjeby
Copy link
Owner

pjeby commented Jul 9, 2022

Okay, try changing the two instances of setImmediate in main.js to Promise.resolve().then, i.e.:

workspace.onLayoutReady(() => setImmediate(() => this.forWindow(win)));

becomes:

workspace.onLayoutReady(() => Promise.resolve().then(() => this.forWindow(win)));

And:

workspace.onLayoutReady(() => setImmediate(() => this.forAll()));

becomes:

workspace.onLayoutReady(() => Promise.resolve().then(() => this.forAll()));

(Also, please use the latest version of the plugin.)

@FelipeRearden
Copy link
Author

FelipeRearden commented Jul 9, 2022

I have very good news :)

Also, please use the latest version of the plugin.

✅ Procedure described below was did at version 0.1.11

Okay, try changing the two instances of setImmediate in main.js to Promise.resolve().then, i.e.:

Thanks for the prefect instructions.

Lines 484 and 486 were changed. The picture below show the main.js AFTER the change:

F69F8B07-F914-4E1D-BA89-039C0EE74733

I humbly ask for you to confirm if I did the right thing (as a non-developer I have fear when I don't know what I am doing)

Results

ℹ️ Before writing this I closed Obsidian and open again and after that I started to use the plugins:

✅ Installation 100% perfect;
✅ Plugin enable 100% perfect;
✅ MAP command working 100% perfect;
✅ PR Commands working 100% perfect;
✅ PR Commands on CPHB are working perfect;
✅ 0% of Console Errors

🆘 CPHB integration

I was able to enable the PR setting on Settings pane but I couldn't see the numbers around the arrows (counters):

54143818-A00D-4E98-A43F-BB6B9698636E

BUT the latest version on CPHB is for 0.15.4 (latest beta) which is different from the latest Mobile version which mirrors 0.14.15

I think we have to wait for the next Mobile version to confirm. But I am more than happy to be able to use PR on Mobile without the numbers :)


Do you think we are gonna have PR on mobile? 👀👀👀 🙏🙏🙏

Thanks for all your help!

@pjeby
Copy link
Owner

pjeby commented Jul 9, 2022

So, I will include the setImmediate fix in the next Pane Relief. But to commit to making it installable without manually changing the manifest.json, I don't think it is something I can do at the moment, since I have no way of knowing if a change I make will break the plugin. And if a change breaks the plugin you will lose history, because history is lost if you close Obsidian or change workspaces without the plugin active. As things stand, mobile users would need to only upgrade the plugin in a test vault and test that all the functionality is working before upgrading in production.

In the long run, my plans for Pane Relief include changing the way it stores things, such that this issue could be worked around. And I'm also okay with continuing to try to keep the plugin running on mobile if you or others want to share reports like this one. But I really don't see how I can make it generally available at the moment such that people who just randomly download the plugin to use on mobile don't end up with problems.

@FelipeRearden
Copy link
Author

FelipeRearden commented Jul 9, 2022

So, I will include the setImmediate fix in the next Pane Relief.

  • Thanks. I will redo the test after the update and report to you.

But to commit to making it installable without manually changing the manifest.json, I don't think it is something I can do at the moment, since I have no way of knowing if a change I make will break the plugin.

100% agree with your approach. It's the best way to keep things working for everybody.

Now we know the point that makes PR working on Mobile and the user decides if he wants to use on Mobile by manually changing the manifest.json (after the setImmediate fix)

I think we are gonna have more people like me testing the behavior or Mobile in the long term and maybe we get an "official" support for Mobile in the future. Lets see :)

@pjeby
Copy link
Owner

pjeby commented Jul 10, 2022

0.1.14 now has the setImmediate change included. Please let me know if it works. Thanks.

@FelipeRearden
Copy link
Author

0.1.14 now has the setImmediate change included. Please let me know if it works. Thanks.

Working perfect on version 0.1.15 on Obsidian Mobile version 1.2.3 on iPadOS 15.4

Thanks @pjeby

@pjeby
Copy link
Owner

pjeby commented Jul 10, 2022

Great. I'm going to close this now; feel free to open new issues if a future update breaks things. 😉

@pjeby pjeby closed this as completed Jul 10, 2022
@FelipeRearden
Copy link
Author

Great. I'm going to close this now; feel free to open new issues if a future update breaks things. 😉

Thanks a lot @pjeby ! I let you know if I get some issues in the future releases of PR :)

@FelipeRearden
Copy link
Author

Jus to let you know that I see that we have a new release for Mobile (1.3.0) that mirrors that new public release 15.6.

I always wait 1/2 weeks after a public release to upstate my main vault:

  • sometimes I have plugins that are not compatible with new versions that breaks my workflow;
  • on Mobile we cant have two vault with different releases/versions. This way I cant have my test vault on 1.3.0 yet.

Just to let you know that I have 👀 on this and I get back to you soon :)

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

No branches or pull requests

2 participants