Page MenuHomePhabricator

Unit test ext.popups.disablenavpop.js
Closed, InvalidPublic

Description

No coverage whatsoever. Unit test properly.

Just a function, 22LOC. Relies in a lot of global state.

Also, please remove // HACK: This is a temporary fix, because it clearly is not. Disabling navigation popups in hovercards is a feature, at least while navigation popups exists.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 19 2016, 9:37 AM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 19 2016, 4:11 PM
Jhernandez updated the task description. (Show Details)Apr 20 2016, 11:10 AM

Also, please remove // HACK: This is a temporary fix, because it clearly is not. Disabling navigation popups in hovercards is a feature, at least while navigation popups exists.

I heavily encourage followup patches after testing to refactor this little piece of code. Could be a good exercise for offsite.

bmansurov updated the task description. (Show Details)Apr 20 2016, 3:36 PM
phuedx closed this task as Invalid.Feb 15 2017, 6:16 AM
phuedx added a subscriber: phuedx.

This code was removed during T149801: [EPIC] Refactor Hovercards (née Popups). Since the rewrite branch has been merged, this task is now invalid.

For posterity, T151058: Allow users who have navigational popups to use hovercards tracked ensuring that NavPopups and Page Previews played nicely together in the rewrite branch.