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.

Sign up for GitHub

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

Jump to bottom

FeatureFormView - Attachment handling revisions #729

Open
wants to merge 29 commits into
base: FeatureFormAttachments
Choose a base branch
from
Open

FeatureFormView - Attachment handling revisions #729

wants to merge 29 commits into from

Conversation

dfeinzimer
Copy link
Collaborator

Related Apollo 684

@dfeinzimer dfeinzimer requested a review from mhdostal May 16, 2024 02:03
@dfeinzimer dfeinzimer self-assigned this May 16, 2024
@dfeinzimer dfeinzimer marked this pull request as draft May 16, 2024 20:05
@dfeinzimer dfeinzimer removed the request for review from mhdostal May 16, 2024 20:05
@dfeinzimer dfeinzimer requested a review from mhdostal May 20, 2024 17:45
@dfeinzimer dfeinzimer marked this pull request as ready for review May 20, 2024 17:45
@dfeinzimer dfeinzimer marked this pull request as draft May 21, 2024 21:39
@dfeinzimer dfeinzimer removed the request for review from mhdostal May 21, 2024 21:40
@dfeinzimer dfeinzimer changed the base branch from FeatureFormAttachments to df/centralizeLocalizedStrings May 31, 2024 00:17
@dfeinzimer dfeinzimer requested a review from mhdostal May 31, 2024 00:44
@dfeinzimer dfeinzimer marked this pull request as ready for review May 31, 2024 00:44
@dfeinzimer
Copy link
Collaborator Author

This is ready for review once again

Base automatically changed from df/centralizeLocalizedStrings to FeatureFormAttachments May 31, 2024 16:35
@dfeinzimer
Copy link
Collaborator Author

Still ready for review. Updated AttachmentFormElement.deleteAttachment(attachment) to AttachmentFormElement.delete(attachment).

@mhdostal
Copy link
Member

Updated AttachmentFormElement.deleteAttachment(attachment) to AttachmentFormElement.delete(attachment).

Is that what the new generated code outputs?

@mhdostal
Copy link
Member

Does the AttachmentImportMenu.init(element: AttachmentsFormElement, onAdd: ((FeatureAttachment) async throws -> Void)? = nil) method still need the onAdd argument to be async throws?

Same question for onDelete and onRename for the AttachmentPreview class?

@dfeinzimer
Copy link
Collaborator Author

Is that what the new generated code outputs?

It looks like taking the attachments property out of generation to make it not-cached actually caused that.. which I wouldn't have expected. Reverting for now.

@dfeinzimer dfeinzimer requested a review from mhdostal May 31, 2024 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdostal mhdostal Awaiting requested review from mhdostal

Assignees

@dfeinzimer dfeinzimer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dfeinzimer @mhdostal