Port RB.MenuButtonView to TypeScript and spina, remove SplitButtonView.

Review Request #12824 — Created Feb. 3, 2023 and submitted

david
Review Board
release-6.x
reviewboard

This change ports the MenuButtonView UI element over to our new
TypeScript/spina framework.

The MenuButtonView had mostly replaced the old SplitButtonView,
which had been marked as deprecated since 4.0 (and which had become a
simple wrapper). As part of this change, I've moved the couple old uses
of SplitButtonView over to use MenuButtonView directly.

  • Verified "Publish" button functionality on the review dialog and draft
    review banner.
  • Ran js-tests.
Summary
Port RB.MenuButtonView to TypeScript and spina, remove SplitButtonView.
Description From Last Updated

Can we make sure we're using one-line summaries for these? These will all eventually go through the same Napoleon doc …

chipx86chipx86

One-line/sentence summary here too. And below.

chipx86chipx86

Blank line between these. Makes it easier to document the instance variables when public. And actually, we should probably stick …

chipx86chipx86

Wonder if we should start making these static. I can't remember if there are currently any issues using that.

chipx86chipx86

I don't think we need to call the parent when subclassing BaseView/View.

chipx86chipx86

No need to call the parent here. It's explicitly empty.

chipx86chipx86

We should use ... as Element instead of <Element>.... This is the recommended syntax in TypeScript going forward (designed to …

chipx86chipx86

Can we sort this?

chipx86chipx86

This shouldn't be indented.

chipx86chipx86

We should also include Version Added here.

chipx86chipx86

We can use the _ template literal stuff for these gettext calls. (_ makes ESLint grumble a bit, but I …

chipx86chipx86

Leftover debugging.

chipx86chipx86

Leftover debugging.

chipx86chipx86

Can we put this in alphabetical order?

chipx86chipx86

These should be combined, updated for bullet points.

chipx86chipx86
chipx86
  1. 
      
  2. Can we make sure we're using one-line summaries for these? These will all eventually go through the same Napoleon doc handling that the Python code goes through, so it'll need to conform to that.

  3. One-line/sentence summary here too.

    And below.

  4. Blank line between these. Makes it easier to document the instance variables when public.

    And actually, we should probably stick to the usual ordering of public and then private.

  5. Wonder if we should start making these static. I can't remember if there are currently any issues using that.

    1. We can do private or we can do static, but static private can't be used with decorators, for some reason. Given the choice, I think I prefer private.

  6. I don't think we need to call the parent when subclassing BaseView/View.

  7. No need to call the parent here. It's explicitly empty.

  8. We should use ... as Element instead of <Element>.... This is the recommended syntax in TypeScript going forward (designed to avoid issues with JSX).

    Here and anywhere else we may be doing it.

  9. reviewboard/static/rb/js/ui/views/menuButtonView.ts (Diff revision 1)
     
     
     
     
     

    Can we sort this?

    1. Yes, but I'm also going to switch back to if statements as mentioned in another change.

  10. This shouldn't be indented.

  11. We should also include Version Added here.

  12. We can use the _ template literal stuff for these gettext calls.

    (_ makes ESLint grumble a bit, but I plan to address that.)

  13. Leftover debugging.

  14. 
      
david
chipx86
  1. 
      
  2. reviewboard/static/rb/js/ui/index.ts (Diff revision 2)
     
     
     

    Can we put this in alphabetical order?

  3. reviewboard/static/rb/js/ui/views/menuView.es6.js (Diff revision 2)
     
     
     
     
     
     
     
     

    These should be combined, updated for bullet points.

  4. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (2d165c6)
Loading...