• 
      

    Port MenuView to TypeScript and spina.

    Review Request #12827 — Created Feb. 7, 2023 and submitted

    Information

    Review Board
    release-6.x

    Reviewers

    This change ports the MenuView over to TypeScript and spina. This is
    relatively straightforward.

    As part of this, the ui bundle has been changed to load on all pages
    instead of just ones inheriting from reviewable_base.html

    • Tested various places that used MenuView (menu actions, menu buttons in
      banners/dialogs).
    • Ran js-tests.
    Summary ID
    Port MenuView to TypeScript and spina.
    This change ports the MenuView over to TypeScript and spina. This is relatively straightforward. As part of this, the `ui` bundle has been changed to load on all pages instead of just ones inheriting from reviewable_base.html Testing Done: - Tested various places that used MenuView (menu actions, menu buttons in banners/dialogs). - Ran js-tests.
    fa42e66f849bab28fadb88dd4a1db2a9eaf39649
    Description From Last Updated

    I don't remember where we landed on this from another review, but do we want to split this into two …

    chipx86chipx86

    Let's alphabetize by import module.

    chipx86chipx86

    Can we type and document these using the interface?

    chipx86chipx86

    Does TypeScript let us use a trailing comma for the last item?

    chipx86chipx86

    Missing Version Added.

    chipx86chipx86

    The summary should be on its own line.

    chipx86chipx86

    Missing a semicolon.

    chipx86chipx86

    Do subclasses need to set these? If not, we can avoid all this and use the proper elements in the …

    chipx86chipx86

    Let's move private after public.

    chipx86chipx86

    Should we make this an interface?

    chipx86chipx86

    The <cast>... syntax is considered deprecated. We should use ... as type instead.

    chipx86chipx86

    Casting should use ... as ... syntax.

    chipx86chipx86
    chipx86
    1. 
        
    2. Show all issues

      I don't remember where we landed on this from another review, but do we want to split this into two import groups, from the perspective of this bundle?

    3. reviewboard/static/rb/js/ui/index.ts (Diff revision 1)
       
       
       
       
      Show all issues

      Let's alphabetize by import module.

    4. reviewboard/static/rb/js/ui/views/menuButtonView.ts (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      Can we type and document these using the interface?

    5. Show all issues

      Does TypeScript let us use a trailing comma for the last item?

    6. reviewboard/static/rb/js/ui/views/menuView.ts (Diff revision 1)
       
       
       
       
      Show all issues

      Missing Version Added.

    7. Show all issues

      The summary should be on its own line.

    8. Show all issues

      Missing a semicolon.

    9. reviewboard/static/rb/js/ui/views/menuView.ts (Diff revision 1)
       
       
       
       
       
      Show all issues

      Do subclasses need to set these? If not, we can avoid all this and use the proper elements in the base class setup.

    10. reviewboard/static/rb/js/ui/views/menuView.ts (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      Let's move private after public.

    11. reviewboard/static/rb/js/ui/views/menuView.ts (Diff revision 1)
       
       
       
       
       
       
       
      Show all issues

      Should we make this an interface?

    12. Show all issues

      The <cast>... syntax is considered deprecated. We should use ... as type instead.

    13. Show all issues

      Casting should use ... as ... syntax.

    14. 
        
    david
    chipx86
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-6.x (7eb202e)