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

Inline rendering only shows the first line of properties in reading view #2370

Closed
reply2za opened this issue Jun 17, 2024 · 10 comments
Closed
Labels
bug Something isn't working. mobile Issues related to Dataview on mobile Obsidian.

Comments

@reply2za
Copy link
Contributor

What happened?

Inline rendering is only showing the first line of properties over and over again in reading view. On the other hand, live preview renders fine. I tried this in an empty vault with the only plugin being the latest version of Dataview.

I did a search and noticed that this is likely a repeat of an existing (old and unclosed) issue. I would like to bring this back up as it hasn't been fixed in 6+ months.

DQL

source view

  • [prop1:: A] [prop2:: watermelon]
  • [prop1:: B] [prop2:: kiwi]
  • [prop1:: C] [prop2:: oranges]

reading view (implied)

  • [prop1:: A] [prop2:: watermelon]
  • [prop1:: A] [prop2:: watermelon]
  • [prop1:: A] [prop2:: watermelon]

JS

No response

Dataview Version

v0.5.66

Obsidian Version

1.6.3

Device

iPhone 15

OS

17

@reply2za reply2za added bug Something isn't working. mobile Issues related to Dataview on mobile Obsidian. labels Jun 17, 2024
@reply2za
Copy link
Contributor Author

This bug is actually not exclusive to mobile, as I also see the same behavior on desktop.

@holroy
Copy link
Contributor

holroy commented Jun 17, 2024

This sounds a lot like ##2216, which sadly I'm not sure if we're able to correct, or whether we need to code some workaround to avoid this happening.

@reply2za
Copy link
Contributor Author

reply2za commented Jun 17, 2024

That's sad to hear as I do prefer reviewing my notes in reading mode... If you are familiar with this issue, can you give a technical rundown of where the issue is stemming or what files are involved in inline rendering. That way more eyes can get up to speed with the issue and thus may bring us closer to a possible fix.

-edit-
I noticed that there is some detail that you provided in the issue you linked. If there isn't any new findings then that's ok. Thanks for having looked into it.

@holroy
Copy link
Contributor

holroy commented Jun 17, 2024

It's documented in that linked issue. The bug(s) was introduced after 0.5.59, and is related to a double rendering of text as markdown, where in the new (and buggy) version it assumes it gets the current line of a list item, but it receives the entire block of the list, which results in a faulty re-rendering.

@holroy
Copy link
Contributor

holroy commented Jun 17, 2024

I've been contemplating to "hide" all the related changes of this bug under some experimental setting within Dataview, but it's a lot of code which needs to be reviewed and handled, and I've not received any response from he who implemented this stuff as to why it was implemented or if he could fix it. So I'm kind of unsure why this code is there in the first place.

@reply2za
Copy link
Contributor Author

reply2za commented Jun 18, 2024

@holroy I found the offending line and was able to fix the issue with some minor changes. I made a fork with the fix. I also made a pull request.

pull request: #2377

@holroy
Copy link
Contributor

holroy commented Jun 18, 2024

That's interesting indeed, and I hope you're right, but are you sure you're preserved the same logic (whatever that was) which was introduced in 0.5.60++?

I'll look into this out of personal interest, but can't do so before in a day or two, but I hope your solution is solid and can be included and solve this long lasting issue.

@reply2za
Copy link
Contributor Author

Yes, so the last time the file was touched 8 months ago by @RyotaUshio. The change essentially reverts his changes in this one file alone and not the other changes he's made to other files. This is straightforward in resolving the rendering issue without impacting anything else.

@holroy
Copy link
Contributor

holroy commented Jun 19, 2024

There is no question that you've found the tricky code spot. I do remember the affected code from my last review of this issue. I just fail at understanding @RyotaUshio purpose with the code change, and how (and where) to implement the solution.

Therefore I find it very interesting to see this change which seems to take a very different approach and implementation of the issue at hand.

So I'm not opposing this PR in any way, I just want to figure out what is happening, how this code resolves it, and also see that my test cases related to this issue are resolved.

@reply2za
Copy link
Contributor Author

reply2za commented Jun 19, 2024

I did some more digging and realized that @RyotaUshio made the change to fix issue #2084. The issue was that latex code does not render in reading view. In fixing this, everything else in reading view broke (i.e. stopped rendering). I reviewed his code thoroughly to understand where it went wrong to keep the fix for #2084. Ultimately, I was able to get it working so both non-latex and latex code both renders in reading view the way it should. Live preview continues to work as per-usual since this code doesn't affect live preview. Furthermore, this change works on mobile too.

This took longer than I expected. I updated the PR and added a comment in the code explaining the snippet that's involved in latex handling for any future maintainers of the code. I hope this gets resolved so inline properties can become usable once again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. mobile Issues related to Dataview on mobile Obsidian.
Projects
None yet
Development

No branches or pull requests

2 participants