Page MenuHomePhabricator

Remove ButtonWithSpinner from MobileFrontend
Closed, ResolvedPublic


Now Gather is being sunsetted this class is not being used by anything and should be removed from the repo.

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Readers-Web-Backlog.
Jdlrobson moved this task to Triaged but Future on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Jdlrobson.

remove the OOjs UI dependency

The point of creating the button was to slowly start switching to oojs-ui.

@bmansurov I think this totally the wrong way to do it. It makes it really difficult for us to move things to stable and it doesn't actually progress us in an way - we cannot load all our View code AND oojs ui it has to be one or the other.

As I've stated so many times T88559 is the best place to start this. The Overlay is our most complicated piece of code and VisualEditorOverlay is already using both libraries but not loading them all on startup and only for tablet. By rewriting this as a dialog we will be able to move to oojs ui fully effortlessly off the back of everything we discover.

Let's be honest moving to oojs ui is not a week worth of work, there's a hell of a lot we have to do before that.

The problem with introducing OO-UI widgets into MF code is that it is a complete anti-pattern to the existing framework.
MF's strongest pattern is the view/template/events map. Constructing widgets after these views are rendered goes entirely against that and makes our code look ugly. @kaldari, @Jdlrobson feel free to correct me / add ideas on what needs to happen to start introducing oo-ui widgets

Change 203863 had a related patch set uploaded (by Jdlrobson):
Hygiene: Write ButtonWithSpinner as a View

Jdlrobson set Security to None.
phuedx added a subscriber: phuedx.

Given the last comment on the patch and my recent conversation with @Jdlrobson about the best first step to integrating OOJS UI, I'd say that this should be closed as Invalid.

This is still a valid bug @phuedx ButtonWithSpinner not being a View is a problem as it blocks T94233: Promote Gather to stable. We can work around the EventEmitter issues but the fact oojs ui is packaged up with all our other code is a problem we'll need to resolve.

We should rewrite the task to be clearer or create a new bug to that effect? What do you prefer?

Change 203863 abandoned by Jdlrobson:
WIP: Hygiene: Write ButtonWithSpinner as a View

I wouldn't say it's a bug for starters ;) Making the task a bit clearer might be useful. I'm not sure why it's in Mobile Web Sprint 45 though…

Jhernandez triaged this task as Medium priority.Aug 18 2015, 5:33 PM
Jhernandez added a subscriber: Jhernandez.
jhobs lowered the priority of this task from Medium to Low.May 25 2016, 9:34 PM
jhobs added a project: Technical-Debt.
Jdlrobson renamed this task from ButtonWithSpinner is not a true View to Remove ButtonWithSpinner from MobileFrontend.May 25 2016, 9:34 PM
Jdlrobson updated the task description. (Show Details)

Change 291783 had a related patch set uploaded (by Bmansurov):
Remove the mobile.buttonWithSpinner module

Change 291783 merged by jenkins-bot:
Remove the mobile.buttonWithSpinner module

Jdlrobson claimed this task.