Page MenuHomePhabricator

Consolidate task card logic
Closed, DeclinedPublic

Description

This came up during the code review of this patch.

make a single helper class for all task card logic (mobile module preview, mobile module, desktop module, post-edit dialog) and cut down on code duplication.

It would be nice to take action on this to reduce code duplication and facilitate maintenance.

Event Timeline

Sgs added a subscriber: Tgr.

I'm understanding what we want to consolidate is non HTML render logic, right? @Tgr

Yeah, we have fairly similar logic in PostEditPanel.getCard(), SuggestedEditsModule.setupClickLogging() + the SuggestedEditCardWidget constructor, and now in loadExtraDataForSuggestedEdits().

In theory we could try to unify the HTML rendering logic in SuggestedEditCardWidget and SmallTaskCard too (they are quite different now, but design-wise the two don't differ that much), and even the PHP task card widget via some sort of templating, but I doubt it's worth the effort, especially with the Vue migration being around the corner.

Yeah, we have fairly similar logic in PostEditPanel.getCard(), SuggestedEditsModule.setupClickLogging() + the SuggestedEditCardWidget constructor, and now in loadExtraDataForSuggestedEdits().

In theory we could try to unify the HTML rendering logic in SuggestedEditCardWidget and SmallTaskCard too (they are quite different now, but design-wise the two don't differ that much), and even the PHP task card widget via some sort of templating, but I doubt it's worth the effort, especially with the Vue migration being around the corner.

I agree this task is not worth right now foreseeing the migration to Vue. Declining it for now.