Page MenuHomePhabricator

Implement large/small buttons in OOUI
Closed, ResolvedPublic1.5 Estimated Story Points

Description

image.png (74×47 px, 1 KB)
image.png (93×68 px, 1 KB)
Implement large and small button sizes (needed in T414518)

Related Objects

Event Timeline

Esanders set the point value for this task to 1.5.

Change #1266271 had a related patch set uploaded (by Esanders; author: Esanders):

[oojs/ui@master] ButtonElement: Add support for small/large buttons

https://gerrit.wikimedia.org/r/1266271

Note there are lots of button variants to support too (icons, indicators, labels can all be turned on and off):

image.png (951×223 px, 20 KB)

For now I've also left out frameless buttons, and also I haven't implemented in Apex (Monobook).

The reason I haven't implemented frameless buttons is because in OOUI frameless buttons already have reduce horizontal padding, so it is not obvious how to adjust those (especially for small buttons).

Change #1266271 merged by jenkins-bot:

[oojs/ui@master] ButtonElement: Add support for small/large buttons

https://gerrit.wikimedia.org/r/1266271

Change #1275892 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/core@master] Upgrade OOUI from v0.53.1 to v0.53.2

https://gerrit.wikimedia.org/r/1275892

Change #1275892 merged by jenkins-bot:

[mediawiki/core@master] Upgrade OOUI from v0.53.1 to v0.53.2

https://gerrit.wikimedia.org/r/1275892

Change #1275905 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/core@master] HTMLFormFieldTestCase: Re-enable testGetInputOOUI now upgrade is done

https://gerrit.wikimedia.org/r/1275905

Change #1275905 merged by jenkins-bot:

[mediawiki/core@master] HTMLFormFieldTestCase: Re-enable testGetInputOOUI now upgrade is done

https://gerrit.wikimedia.org/r/1275905

Change #1275975 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/DiscussionTools@master] tests: Regenerate cases for new OOUI medium button size class

https://gerrit.wikimedia.org/r/1275975

Change #1275975 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] tests: Regenerate cases for new OOUI medium button size class

https://gerrit.wikimedia.org/r/1275975

Change #1276022 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/DiscussionTools@master] tests: Regenerate cases for new OOUI medium button size class in mobile format too

https://gerrit.wikimedia.org/r/1276022

@Jdforrester-WMF, that DiscussionTools patch does not fix CheckUser CI. Is something else needed?

@Jdforrester-WMF, that DiscussionTools patch does not fix CheckUser CI. Is something else needed?

I'm minded to just drop their tests to @group Broken. These tests shouldn't be so fragile, and probably should just be in a standalone group anyway.

Change #1276069 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/DiscussionTools@master] tests: Just disable CommentFormatterTest for now

https://gerrit.wikimedia.org/r/1276069

Change #1276022 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] tests: Regenerate cases for new OOUI medium button size class in mobile format too

https://gerrit.wikimedia.org/r/1276022

@Jdforrester-WMF, that DiscussionTools patch does not fix CheckUser CI. Is something else needed?

I'm minded to just drop their tests to @group Broken. These tests shouldn't be so fragile, and probably should just be in a standalone group anyway.

We shouldn't disable these tests. The last couple of times they broke CI, they alerted us about real problems that could have broken DiscussionTools (T418982 was a bug in ParserMigration; T417994 and T413573 was an incompatibility with PHP 8.4; and while I can't cite examples readily, they have definitely prevented bugs in DiscussionTools changes too). The code they cover is not covered by any better tests. This is not great, but we live with much shoddier test suites. I think they add value overall.

(And IMO this failure should have blocked the merge of the OOUI bump in MediaWiki, but that's T249674.)

Why does CheckUser run the DiscussionTools test suite, anyway? Maybe that can be changed.

Why does CheckUser run the DiscussionTools test suite, anyway? Maybe that can be changed.

T396709: UserInfoCard: Display icon button in DiscussionTools.

Change #1276074 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/config@master] Zuul: [mediawiki/extensions/DiscussionTools] Add standalone test jobs

https://gerrit.wikimedia.org/r/1276074

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1180181 is now failing with a GlobalBlocking problem:

1) MediaWiki\Extension\GlobalBlocking\Test\Unit\Special\SpecialGlobalBlockStatusTest::testDoesWrites
Use of MediaWiki\SpecialPage\SpecialPage constructor parameters $restriction, $listed, $function, $file and $includable was deprecated in MediaWiki 1.46. [Called from MediaWiki\Extension\GlobalBlocking\Special\SpecialGlobalBlockStatus::__construct in /workspace/src/extensions/GlobalBlocking/includes/Special/SpecialGlobalBlockStatus.php at line 35]

Maybe the CI issue should have been a separate task…

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/1180181 is now failing with a GlobalBlocking problem:

1) MediaWiki\Extension\GlobalBlocking\Test\Unit\Special\SpecialGlobalBlockStatusTest::testDoesWrites
Use of MediaWiki\SpecialPage\SpecialPage constructor parameters $restriction, $listed, $function, $file and $includable was deprecated in MediaWiki 1.46. [Called from MediaWiki\Extension\GlobalBlocking\Special\SpecialGlobalBlockStatus::__construct in /workspace/src/extensions/GlobalBlocking/includes/Special/SpecialGlobalBlockStatus.php at line 35]

Maybe the CI issue should have been a separate task…

I'm handling this one in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GlobalBlocking/+/1276073

Change #1276074 merged by jenkins-bot:

[integration/config@master] Zuul: [mediawiki/extensions/DiscussionTools] Add standalone test jobs

https://gerrit.wikimedia.org/r/1276074

Change #1276076 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/DiscussionTools@master] tests: Split out Comment* tests to standalone

https://gerrit.wikimedia.org/r/1276076

Mentioned in SAL (#wikimedia-releng) [2026-04-21T23:17:32Z] <James_F> Zuul: [mediawiki/extensions/DiscussionTools] Add standalone test jobs, for T422031

Why does CheckUser run the DiscussionTools test suite, anyway? Maybe that can be changed.

T396709: UserInfoCard: Display icon button in DiscussionTools.

Okay, so:

CheckUser's new DiscussionToolsSignatureHandler implements a hook
interface from DiscussionTools, so it needs to be available for
both test runs and phan static analysis.

This can be achieved with stubs for Phan (example) and by mocking the HookRunner in tests.

I also note that none of the code from that task that would make CheckUser depend on DiscussionTools is actually merged, so this just makes your test suite slower for no benefit…

I also note that none of the code from that task that would make CheckUser depend on DiscussionTools is actually merged, so this just makes your test suite slower for no benefit…

I would note that CheckUser also depends on VisualEditor which depends on DiscussionTools, so that patch didn't actually start loading those tests in CheckUser CI but formalised the dependency being needed. This is because CheckUser is still set to recursively get extensions due to waiting for other extensions to fix their tests

Perhaps a phan stub would be okay or simply reverting the CI config change for now, but that doesn't fix the issue in CheckUser for now as mentioned above

Change #1276079 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] Regenerate mobile test cases again with the right results

https://gerrit.wikimedia.org/r/1276079

Change #1276069 abandoned by Jforrester:

[mediawiki/extensions/DiscussionTools@master] tests: Just disable CommentFormatterTest for now

https://gerrit.wikimedia.org/r/1276069

Change #1276079 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Regenerate mobile test cases again with the right results

https://gerrit.wikimedia.org/r/1276079

Maybe the CI issue should have been a separate task…

Yeah, I was thinking something like that as well tbh

Change #1276076 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] tests: Split out Comment* tests to standalone

https://gerrit.wikimedia.org/r/1276076