-
Notifications
You must be signed in to change notification settings - Fork 66
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
Ability to Set Post Language and Edit Existing Posts #893
Conversation
@micahmo Feel free to review the PR if you'd like or do some testing! I think most cases were caught, but since this is a fairly big refactor, some things may have been missed (which should hopefully be caught when this is released to the nightly builds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, I especially like the draft refactoring!
|
||
// used create post from action sheet | ||
/// Whether or not to pre-populate the post with the [title], [text], [image] and/or [url] | ||
final bool? prePopulated; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if we might want to think about consolidating all the different ways we can prepopulate a post. Right now I think we have drafts, edits, and shares, each with different things that are saved/restored. (For example, is language selection saved with drafts?) Maybe we can have a wrapper class around a Post/View/Media/whatever
, so that it can contain every possible field, and then an enum saying what kind of thing it is. Just a thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good idea! I think we could do this in a different PR since this one already contains a pretty substantial change if thats all good with you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course!
Pull Request Description
This is a PR which adds the ability to edit existing posts, and set the specified language for the post. It also adds a few UI changes to make the experience a bit more cohesive. The
CreatePostPage
was fully refactored to be able to take advantage of some of the comments mentioned in #853 (comment). This includes:CreatePostPage
. This means we no longer need the previous draft logic outside of theCreatePostPage
. This does not apply to the comment page as that still retains the original logic.context
)Notes:
Issue Being Fixed
Issue Number: #157, #892
Screenshots / Recordings
Screen.Recording.2023-11-14.at.7.34.20.PM.mov
Checklist
semanticLabel
s where applicable for accessibility?