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

Implement Modal Editor Sheet #1141

Merged
merged 17 commits into from
Mar 27, 2024
Merged

Implement Modal Editor Sheet #1141

merged 17 commits into from
Mar 27, 2024

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented Mar 21, 2024

Fixes #1057

Preserves existing editor as-is unless the Developer Setting is enabled.

Next up: integrate into all views / contexts.

Tasks

  • Add developer flag to enable/disable
  • Cleanup code
  • Modify audience from inner sheet
  • Modify link from inner sheet
  • Post / save on button, auto assign audience
  • Explore animation performance with large text
Screen.Recording.2024-03-22.at.11.52.58.AM.mov

Comment on lines +21 to +23

var background = Color.background
var color = Color.accentColor
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Allow theming the keyboard toolbar to match content

import Combine

// MARK: View
struct EditorModalSheetDetailView: View {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I forked the existing EditorDetailView for the modal sheet, it's powered by the same underlying model.

When we're ready to cut over we can flatten this out and remove the unused features.

}
}

struct EditorModalSheetModel: ModelProtocol, Equatable {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a very thin model, I imagine it will merge with the main editor model in the future.

EdgeInsets(
top: DeckTheme.cardPadding,
leading: DeckTheme.cardPadding,
bottom: 2 * DeckTheme.cardPadding,
Copy link
Collaborator Author

@bfollington bfollington Mar 22, 2024

Choose a reason for hiding this comment

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

Space for the "swipe up" system bar

Comment on lines +20 to +22
if app.state.isModalEditorEnabled {
app.send(.editorSheet(.editEntry(entry)))
} else {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the setting is enabled notes edited from the notebook will launch in the sheet.

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 as a first pass.

Before unflagging, I'd like us to do some visual/UX refinements

  • Refining spacing of header
    • Match the spacing of the native sliding panels header
    • We could make this a re-usable SwiftUI view
  • Use disc style for close button, ensure we match native dimensions
    • At one point we had a view for this. Maybe still do?
  • Auto-focus editor
    • ideally on the character position, or at least the block that is tapped in view-mode
  • Refine Post/save flow
    • I think we want to have something similar to a social media sheet.
    • When you create a new note (draft), header says "post".
    • If you exit, the note is saved as a draft
      • We may want to introduce an ephemeral draft state with a nag for "discard changes" or "save draft" to simplify mental model.
    • If you hit "post", the note is published.
    • After a note is published, the button changes to "save"
      • If we introduce the save nag, exiting would say "Discard changes? Yes/cancel"

Let me know if dropping into figma would make these notes clearer.

@bfollington
Copy link
Collaborator Author

@gordonbrander Just FYI, as part of this PR I've addressed:

  • Refining spacing of header
  • Use disc style for close button, ensure we match native dimensions

and, RE: Refine Post/save flow, I believe it already works the way you're describing with the additional layer of "Save" only appears when there are currently unsaved changes to a public note.

@bfollington bfollington merged commit e64cb55 into main Mar 27, 2024
1 check passed
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.

Mini editor sheet for notes
2 participants