Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refector: MainMenuHandler: action handlers takes ActionEvent object #1055

Conversation

miurahr
Copy link
Member

@miurahr miurahr commented Jun 12, 2024

Pull request type

  • Other (describe below)
    refactor

Which ticket is resolved?

What does this PR change?

  • MainMenuHandler: avoid direct reference to real MainWindow object when getting a menu item status of action event occurred, by passing ActionEvent object to handler method, by using ActionEvent#getSource.
  • Make static fields of Core as a private to allow further refactoring.
  • Instantiate MainWindowMenu in Core and put it in private field
  • Add Core#getMainMenu and change MainWindow#getMainMenu to proxy it

Other information

This comment was marked as resolved.

@miurahr miurahr marked this pull request as ready for review June 12, 2024 22:51
@miurahr miurahr changed the title Refector: MainMenuHanlder: avoid reference to a MainWindow object to improve object-oriented paradigm Refector: MainMenuHandler: avoid reference to a MainWindow object to improve object-oriented paradigm Jun 13, 2024
miurahr added 6 commits June 14, 2024 14:05
- extend invokeAction to take ActionEvent
- extend several methods

Signed-off-by: Hiroshi Miura <[email protected]>
- unbundle a menu object from MainWindow class for unit test

Signed-off-by: Hiroshi Miura <[email protected]>
- hide field and add getters
- allow a future refactoring

Signed-off-by: Hiroshi Miura <[email protected]>
@miurahr miurahr force-pushed the topic/miurahr/editor/menu-item-by-editor-properties-listener branch from fdde568 to 6645fd0 Compare June 14, 2024 05:07
@miurahr miurahr changed the title Refector: MainMenuHandler: avoid reference to a MainWindow object to improve object-oriented paradigm Refector: MainMenuHandler: give ActionEvent object to action handlers Jun 14, 2024
@miurahr miurahr changed the title Refector: MainMenuHandler: give ActionEvent object to action handlers Refector: MainMenuHandler: action handlers takes ActionEvent object Jun 14, 2024
Move initMainMenu static method from Core to MainWindowUI utility class.

Signed-off-by: Hiroshi Miura <[email protected]>
miurahr added 5 commits June 15, 2024 10:09
- All handler takes ActionEvent as argument
- Rewrite invokeAction call by new one
- Keep backward compatible invokeAction(String action, int modifier)

Signed-off-by: Hiroshi Miura <[email protected]>
- Replace mainWindow object access in menu handler with Core.getMainWindow()
- Add default MainWindowMenuHandler constructor w/o argument
- Remove mainWindow field in handler
- Remove redundant override in test codes

Signed-off-by: Hiroshi Miura <[email protected]>
- MainWindowUI.createSearchWindow
- MainWindowUI.findInProjectReuseLastWindow
- MainWindowUI.getTrimmedSelectedTextInMainWindow
- MainWindowUI.projectExit
- MainWindowUI.projectRestart
- MainWindowUI.prepareForExit

Signed-off-by: Hiroshi Miura <[email protected]>
- use MainWindowMenuHandler for the test

Signed-off-by: Hiroshi Miura <[email protected]>
- drop BaseMainWindowMenuHandler because acceptance test
can be implemented with MainWindowMenuHandler

Signed-off-by: Hiroshi Miura <[email protected]>
@miurahr
Copy link
Member Author

miurahr commented Jun 15, 2024

There are too many refactoring in single PR. It is required to split into series of Pull-Requests.

@miurahr miurahr marked this pull request as draft June 15, 2024 03:21
@miurahr
Copy link
Member Author

miurahr commented Jun 16, 2024

this is over-engineered, so I think I can close here and raise another PR in proper size.

@miurahr miurahr closed this Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant