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

Improve deck loading state #1055

Merged
merged 17 commits into from
Jan 9, 2024
Merged

Improve deck loading state #1055

merged 17 commits into from
Jan 9, 2024

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented Jan 5, 2024

Fixes #1052
Fixes #1053
Fixes #1054
Fixes #1024

  • New loading animation for deck
  • Empty state for deck
  • Include links from body of a note when swiping
    • I thought we had this already but it never landed
  • Cards capped to max height
  • Excerpt length increased
  • Text truncated using gradient
Screen.Recording.2024-01-05.at.4.21.29.pm.mov
Simulator Screenshot - iPhone 15 Pro - 2024-01-08 at 15 41 52 Simulator Screenshot - iPhone 15 Pro - 2024-01-08 at 15 32 51 Simulator Screenshot - iPhone 15 Pro - 2024-01-08 at 15 32 42 Simulator Screenshot - iPhone 15 Pro - 2024-01-08 at 15 32 14 Simulator Screenshot - iPhone 15 Pro - 2024-01-08 at 15 31 51 Simulator Screenshot - iPhone 15 Pro - 2024-01-08 at 15 31 25

@bfollington bfollington force-pushed the 2024-01-05-deck-loading-state branch from b4d34a0 to 5dea710 Compare January 8, 2024 02:37
@bfollington bfollington changed the base branch from main to 2023-12-15-intertab-notificiations January 8, 2024 05:14
}
.padding(DeckTheme.cardPadding)
.foregroundStyle(highlight)
.font(.subheadline)
.background(Color.brandMarkPink.opacity(0.025).blendMode(.plusLighter))
Copy link
Collaborator Author

@bfollington bfollington Jan 8, 2024

Choose a reason for hiding this comment

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

Optional: slightly tint the header of prompt cards

Simulator Screenshot - iPhone 15 Pro - 2024-01-08 at 15 31 25

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we lightening the header, or darkening the body?

Comment on lines -21 to -22
// Is the excerpt shorter than the full body of the entry?
let isTruncated: Bool
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We no longer need to store this on the model if we truncate based on rendered size instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we may want to keep this field. I suspect there will be cases where we want to display "read more", or where the gradient treatment may not be ideal.

Not a strong opinion though.

Copy link
Collaborator Author

@bfollington bfollington Jan 9, 2024

Choose a reason for hiding this comment

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

I think we'd be better off determining if the text is truncated in a given view which we can actually do with the gradient truncation helper I built - we can definitely explore a "show more..." on top of the gradient cutoff.

There's some nuance around whitespace and empty blocks that made the old approach (checking string length in the DB query) mismatch with user expectations in the UI - even if we don't use gradients everywhere I'd personally rather count pixels than characters.

@bfollington bfollington self-assigned this Jan 8, 2024
@bfollington bfollington marked this pull request as ready for review January 8, 2024 06:27
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.

LGTM

}
.padding(DeckTheme.cardPadding)
.foregroundStyle(highlight)
.font(.subheadline)
.background(Color.brandMarkPink.opacity(0.025).blendMode(.plusLighter))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we lightening the header, or darkening the body?

Comment on lines -21 to -22
// Is the excerpt shorter than the full body of the entry?
let isTruncated: Bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we may want to keep this field. I suspect there will be cases where we want to display "read more", or where the gradient treatment may not be ideal.

Not a strong opinion though.

xcode/Subconscious/Shared/Services/DatabaseService.swift Outdated Show resolved Hide resolved
@bfollington bfollington force-pushed the 2024-01-05-deck-loading-state branch from c676614 to 9c2cad5 Compare January 9, 2024 04:20
@bfollington bfollington changed the base branch from 2023-12-15-intertab-notificiations to main January 9, 2024 04:21
@bfollington bfollington merged commit 139c857 into main Jan 9, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants