Page MenuHomePhabricator

Rename CardLayout to TabPanelLayout
Closed, ResolvedPublic

Description

After the hint by @Amire80, @Volker_E says that the CardLayout doesn't really represent the user interface concept "card" and it should be called something else. We were thinking "TabPanelLayout"?

Process

  1. Rename CardLayoutTabPanelLayout
    1. Create a new CardLayout that just inherits TabPanelLayout
    2. Rename a bunch of properties and methods in IndexLayout
    3. Add deprecation warnings for everything
  2. Update documentation on Mediawiki: Index and Cards and Layouts.
  3. Wait for, or fix projects that are using the old name (current usage)
    1. T164903: VisualEditor: Use TabPanelLayout instead of CardLayout
    2. T164901: Score: Use TabPanelLayout instead of CardLayout in ve.ui.MWScoreInspector.js
    3. T164904: Citoid: Use TabPanelLayout instead of CardLayout in ve.ui.CiteFromIdInspector.js
    4. T164902: Math: Use TabPanelLayout instead of CardLayout in ve.ui.MWLatexDialog.js
    5. T165927: ContentTranslation: Use TabPanelLayout instead of CardLayout in mw.cx.tools.NewLinkTool.js
  4. Remove CardLayout

Event Timeline

@TrevorParscal Do you remember why it was called that? The change https://gerrit.wikimedia.org/r/#/c/208077/ doesn't offer any hints.

After talking to @TrevorParscal it seems fully appropriate to rename it. He said, that back then it seemed like the right name and the UI concept “card” might not have been that popular.

@Volker_E @matmarex How do we want to go about doing this? I was thinking:

  1. Make a new TabPanelLayout that just calls CardLayout.
  2. Deprecation warning when something calls CardLayout directly.
  3. Fix places that are currently using CardLayout:
    1. Visual Editor
    2. Score
    3. Citoid
    4. Math
  4. Remove CardLayout and rename everything to TabPanelLayout.
  5. ???
  6. PROFIT!!1!

That sounds reasonable. We could rename CardLayout → TabPanelLayout first (and then create a new CardLayout that just inherits TabPanelLayout with a deprecation warning), so that we avoid the old name in our own code during the transition period. And we could keep that alias around for a long time (and let everyone deal with the warnings themselves), since it'd be a tiny cost to support. (We still have a TextInputMenuSelectWidget as alias for FloatingMenuSelectWidget, which was renamed around v0.15 or something. We should probably add deprecation warnings to that, heh.)

Both sounds very reasonable, @Prtksxna's proceeding order and the additional inputs by @matmarex. @Prtksxna are you taking this on?

The IndexLayout is full of references too cards, card, CardLayout...

So IndexLayout has a bunch of methods that use the word card (getCard, removeCard...). In the first step do we also want to add new methods like getTabPanel and issue a deprecation warning when the older method is used? Should we also update all documentation to point to these new methods? I think we should.

Also, what should we do about property names like currentCardName and cards. Can't have a deprecation warning if those are being accessed directly, and its kind of hacky to have two properties that'll have to maintain the same value.

Change 337220 had a related patch set uploaded (by Prtksxna):
[wip] rename cardlayout

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

@Prtksxna your outlined path looks good to me 👍🏻

Also, what should we do about property names like currentCardName and cards. Can't have a deprecation warning if those are being accessed directly, and its kind of hacky to have two properties that'll have to maintain the same value.

@matmarex thoughts?

You could use Object.defineProperty to define them with an getter method, and emit deprecation warning inside that method. (A custom getter/setter will also allow us to magically keep value for the old and new property name in sync.) mw.log.deprecate in MediaWiki does that (https://phabricator.wikimedia.org/source/mediawiki/browse/master/resources/src/mediawiki/mediawiki.js;f1284cbadae948be64e1136385890dd5df9073df$469).

Change 337220 merged by jenkins-bot:
[oojs/ui@master] [DEPRECATING CHANGE] Rename CardLayout to TabPanelLayout

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

Volker_E renamed this task from Rename CardLayout to TabPanelLayout (?) to Rename CardLayout to TabPanelLayout.May 9 2017, 11:55 PM
Volker_E moved this task from Next-up to OOjs-UI-0.21.3 on the OOUI board.
Volker_E edited projects, added OOUI (OOjs-UI-0.21.3); removed OOUI.
Volker_E removed a project: Patch-For-Review.
Volker_E updated the task description. (Show Details)
Volker_E removed a subscriber: gerritbot.

Change 353115 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[oojs/ui@master] demos: Rename deprecated Card to current TabPanel

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

Change 353115 merged by jenkins-bot:
[oojs/ui@master] demos: Rename deprecated Card to current TabPanel

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

Update documentation on Mediawiki: Index and Cards and Layouts.

@Volker_E we might need a redirect for 'Index and Cards'

@Prtksxna Article was moved, so it has an automatic redirect?!

@Prtksxna Article was moved, so it has an automatic redirect?!

You're right. I am sorry, I was only looking at the ToC on the right. Fixed that too.

And we could keep that alias around for a long time (and let everyone deal with the warnings themselves), since it'd be a tiny cost to support.

Given that nothing is using it now, should we decide a date when we remove it?

@Prtksxna Earliest possible release would be v0.23.0.

@Prtksxna Earliest possible release would be v0.23.0.

Not looking for the earliest, but the most appropriate. Could be the same .

Change 364448 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[oojs/ui@master] [BREAKING CHANGE] Remove CardLayout

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

Change 364448 merged by jenkins-bot:
[oojs/ui@master] [BREAKING CHANGE] Remove CardLayout and references in IndexLayout

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

Volker_E removed a project: Patch-For-Review.
Volker_E updated the task description. (Show Details)
Volker_E updated the task description. (Show Details)