Skip to content

Conversation

@Yarloy037
Copy link
Contributor

Description

This PR fixes the event handler syntax in both TypeScript and JavaScript application templates to comply with UI5 best practices and UI5 Linter requirements.

Problem

Generated applications contain event handlers without the required dot prefix:

  • press="sayHello"

This triggers UI5 Linter warning: no-ambiguous-event-handler

Solution

Added dot prefix to all controller method event handlers:

  • press=".sayHello"

Changes

  • Updated resources/template-ts/webapp/view/Main.view.xml
  • Updated resources/template-js/webapp/view/Main.view.xml

References


**Thank you for your contribution!** 🙌

To get it merged faster, kindly review the checklist below:

## Pull Request Checklist
- [ ] Reviewed the [Contributing Guidelines](https://github.com/UI5/mcp-server/blob/main/CONTRIBUTING.md#-contributing-code)
    + Especially the [How to Contribute](https://github.com/UI5/mcp-server/blob/main/CONTRIBUTING.md#how-to-contribute) section 
- [ ] [No merge commits](https://github.com/UI5/mcp-server/blob/main/docs/Guidelines.md#no-merge-commits)
- [ ] [Correct commit message style](https://github.com/UI5/mcp-server/blob/main/docs/Guidelines.md#commit-message-style)

- Add dot prefix to 'sayHello' event handlers in both TS and JS templates
- Fixes UI5 Linter warning: no-ambiguous-event-handler
- Ensures generated apps follow UI5 best practices
@cla-assistant
Copy link

cla-assistant bot commented Dec 1, 2025

CLA assistant check
All committers have signed the CLA.

@Yarloy037 Yarloy037 marked this pull request as draft December 2, 2025 03:31
@Yarloy037 Yarloy037 marked this pull request as ready for review December 2, 2025 03:34
@d3xter666 d3xter666 requested a review from a team December 3, 2025 07:18
@flovogt
Copy link
Member

flovogt commented Dec 3, 2025

@akudev As far as I know, using the heading dot is possible since the early days of UI5 and therefore also supported with older UI5 releases. With 1.136 the global lookup support was deprecated and therefore removed from legacy free versions

@flovogt flovogt requested a review from akudev December 3, 2025 07:50
@flovogt
Copy link
Member

flovogt commented Dec 3, 2025

Global lookup was deprecated with UI5/openui5@915d7eb

@flovogt
Copy link
Member

flovogt commented Dec 3, 2025

@Yarloy037 could you please adjust also the view in the TS template https://github.com/UI5/mcp-server/blob/main/resources/template-ts/webapp/view/Main.view.xml?

@Yarloy037
Copy link
Contributor Author

@Yarloy037 could you please adjust also the view in the TS template https://github.com/UI5/mcp-server/blob/main/resources/template-ts/webapp/view/Main.view.xml?

@flovogt Thank you so much for your review and feedback! 🙏

I'd like to confirm that the TypeScript template (resources/template-ts/webapp/view/Main.view.xml) has already been included in this PR alongside the JavaScript template. Both files have been updated to use the dot prefix syntax for event handlers.

Changes made:

  • resources/template-ts/webapp/view/Main.view.xml - 2 instances (lines ~24 & ~41)
  • resources/template-js/webapp/view/Main.view.xml - 2 instances (lines ~24 & ~41)

You can verify these changes in the "Files changed" tab of this PR. All four occurrences of press="sayHello" have been updated to press=".sayHello".

Please let me know if there's anything else I should address or if any clarification is needed. Thanks again for taking the time to review! 😊

@flovogt
Copy link
Member

flovogt commented Dec 3, 2025

@Yarloy037 Sorry I missed the second file. Thanks a lot for your contribution. I will follow-up with the team on this PR and do a review

@flovogt
Copy link
Member

flovogt commented Dec 3, 2025

@Yarloy037 tests are failing. Could you please adjust them too?

@akudev
Copy link
Member

akudev commented Dec 3, 2025

Yes, this test totally makes sense, thanks. Just needs the test adapted. Try running the unit-update-snapshots package.json script.

@matz3
Copy link
Member

matz3 commented Dec 3, 2025

I've updated the test expectation. Thank you @Yarloy037 for this contribution!

@matz3 matz3 requested a review from RandomByte December 3, 2025 16:17
@matz3 matz3 merged commit d554850 into UI5:main Dec 3, 2025
15 checks passed
@openui5bot openui5bot mentioned this pull request Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants