Skip to content
This repository has been archived by the owner on Aug 11, 2024. It is now read-only.

Render transclude blocks in MemoDetailViewer #574

Merged
merged 23 commits into from
Jun 27, 2023

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented May 5, 2023

Related to #328
Resolves #324
Fixes #710

This approach is based on my experiments with rendering in the editor: #428

Changes

  • Detect slashlinks in subtext blocks
  • Fetch transclude previews from the sphere index (SQLite)
  • Display inline between blocks
  • Translate addresses to match the context
  • Tests for fetching local transcludes
    • Difficult to construct a test for 3P content with transcludes

Own content, own slashlink

image

3P Content, their own slashlink

image

TODO

  • Tests for TranscludeService
    • Insert some notes
    • Force indexing them
    • Call .fetchTranscludes with expected links
    • Try different owner values?
  • Use .findAndPushDetail

@bfollington bfollington changed the title Render transclude blocks in MemoViewer Render transclude blocks in MemoDetailViewer May 5, 2023
@bfollington bfollington marked this pull request as ready for review May 5, 2023 05:32
Copy link
Collaborator

@gordonbrander gordonbrander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Will need to update DB query to match new schema introduced in #561.

@bfollington
Copy link
Collaborator Author

Waiting for #627 to continue pushing this along

@bfollington bfollington force-pushed the 2023-05-05-viewer-transcludes branch from 14f4d3e to 75c56c7 Compare June 23, 2023 04:02
@bfollington bfollington force-pushed the 2023-05-05-viewer-transcludes branch from bbaca87 to 94c15a9 Compare June 26, 2023 01:08
Comment on lines 79 to +81
)
sleep(seconds)
try await Task.sleep(for: .seconds(seconds))

Copy link
Collaborator Author

@bfollington bfollington Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR but I realised this was causing blocking issues while waiting for the gateway to provision.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Namely, it was causing this issue #710

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

Comment on lines +228 to +235
func relativizeIfNeeded(petname base: Petname?) -> Slashlink {
switch self.peer {
case .petname(let name) where name == base:
return Slashlink(slug: self.slug)
default:
return self
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the DID version, drop the peer from the address if it matches the base.

Comment on lines -2519 to +2526
kind = upToNextMajorVersion;
minimumVersion = 2.0.0;
kind = exactVersion;
version = 2.3.2;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinned to exact version to stop the churning on Package.resolved

case you
case ourself
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the noise on this, I think it's better to match the general convention of "ours" instead of "you"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I like it.

@bfollington bfollington force-pushed the 2023-05-05-viewer-transcludes branch from 0ddab7f to 8b8fd25 Compare June 26, 2023 08:19
Copy link
Collaborator

@gordonbrander gordonbrander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve!

Nothing hard blocking. One comment about owner not being used that is worth addressing.

case you
case ourself
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I like it.

Comment on lines 79 to +81
)
sleep(seconds)
try await Task.sleep(for: .seconds(seconds))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

@bfollington bfollington merged commit 115a4ab into main Jun 27, 2023
2 checks passed
bfollington added a commit that referenced this pull request Jun 29, 2023
Since we merged
#574 for
playtesting this PR addresses the formatting and small fixups from
@gordonbrander's review.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gateway features should be disabled before gateway is ready Cross-sphere transcludes
2 participants