feat(eslint-pligin-fiori-tools): report page section name in table rules#4572
feat(eslint-pligin-fiori-tools): report page section name in table rules#4572AlinaGovoruhina wants to merge 22 commits intomainfrom
Conversation
AlinaGovoruhina
added
the
eslint-plugin-fiori-tools
🦋 Changeset detectedLatest commit: 6c17091 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![hyperspace-insights[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR introduces a useful feature but has several issues: a typo producing "settingis disabled" in the table-personalization message, a dead if (copyIssue) guard in sap-copy-to-clipboard.ts, a minor JSDoc typo ("oject"), and — most critically — both processV2Apps and processV4Apps in sap-creation-mode-for-table.ts now silently skip list-report pages because the refactoring only handles object-page sections, which is a regression compared to the original behavior.
| function createTableSection( | ||
| facets: IndexedAnnotation, | ||
| index: number, | ||
| referencedEntityType: string, | ||
| qualifier: string | undefined, | ||
| config: SectionConfig, | ||
| annotationPath: string, | ||
| aliasInfo: AliasInformation, | ||
| service: ParsedService | ||
| ): TableSectionNode | undefined { |
There was a problem hiding this comment.
I prefer object as param of method as this is easier to extend. here we only did for config to keep changes low :) would be nice if we could have one object as param :). Not necessary in this PR as changes might be a lot.
| node, | ||
| messageId: COPY_TO_CLIPBOARD, | ||
| data: { | ||
| sectionText: diagnostic.pageSectionName ? `${diagnostic.pageSectionName} ` : '' |
There was a problem hiding this comment.
See previous comment? Can we have empty pageSectionName? if yes, we can not make it optional. In that case instead of ternary operator will this not be enough. diagnostic.pageSectionName??'' or
`${diagnostic.pageSectionName ?? ''}`
There was a problem hiding this comment.
There can be no sectionName in case of a list report table. Also, to get sectionText we need to add a space after section name. That's why I modified rule unit tests to check than a message text is not adding or missing spaces in cases when there is or isn't a section name.
marufrasully
left a comment
marufrasully
left a comment
There was a problem hiding this comment.
@AlinaGovoruhina thanks.
- review comment addressed
- unit tested
|
2 participants
Report Page Section Name in ESLint Plugin Fiori Tools Table Rules
New Features
✨ ESLint rule diagnostic messages for table-related rules in the
eslint-plugin-fiori-toolspackage now include the page section name when the table is located inside an Object Page section. This provides more precise and context-aware error messages to help developers identify which specific section's table is misconfigured.Changes
diagnostics.ts: Added optionalpageSectionName?: stringfield to several diagnostic interfaces:WidthIncludingColumnHeaderDiagnostic,CreationModeForTable,CopyToClipboard,EnableExport,EnablePaste,TablePersonalization, andCondensedTableLayout.annotations.ts: ExtendedAnnotationBasedNodewith an optionallabelfield. Section label is now extracted fromUI.FacetsannotationPropertyValuerecords and passed intocreateTableSectionandaddHeaderSection, which in turn populatelabelon the resulting nodes.fe-v2.ts/fe-v4.ts: Added optionallabel?: stringto theAnnotationBasedNodeinterface in both V2 and V4 linkers.sap-copy-to-clipboard.ts: Unified V2/V4 handling into a singlehandleCopyInTablefunction and a sharedcheckConfigurationhelper. Object page sections are now iterated to passtableSection.annotation?.labelaspageSectionName. Error message updated to include{{sectionText}}.sap-creation-mode-for-table.ts: UpdatedreportDiagnostic,validateCreateModeV2,processTableV2,validateCreationModeV4, andprocessTableV4to accept and propagatepageSectionName. Object page table iteration now uses sections instead of flat lookup. Error messages updated to include{{sectionText}}.sap-enable-export.ts: RefactoredhandleExportInTableV4to distinguish list report and object page sections. ExtractedcheckConfigurationhelper withpageSectionNamesupport. Message now uses{{sectionText}}.sap-enable-paste.ts: Same refactoring pattern assap-enable-export.ts, with section-aware iteration and updated message template.sap-table-personalization.ts: ExtractedcheckTableConfigurationhelper that passestableSection.annotation?.labelfor object pages.checkPersonalizationValuenow acceptspageSectionNameand propagates it to all reported diagnostics. Error messages updated.sap-width-including-column-header.ts: ExtractedcheckTablehelper withtableSectionNamesupport.checkTablesInPagenow handles list report and object page separately. Message changed to use{{sectionText}}instead of table annotation path.annotation.xml(test data): Fixed a typo ("Prducts"→"Products") and minor formatting cleanup.test-helper.ts: Fixed label typo inV4_FACETS_ANNOTATIONS("Prducts"→"Products").sap-copy-to-clipboard.test.ts,sap-enable-export.test.ts,sap-enable-paste.test.ts,sap-table-personalization.test.ts,sap-width-including-column-header.test.ts: Updated existing tests to assert full messages (including section names) instead ofmessageId, and added new invalid test cases covering object page section scenarios.📬 Subscribe to the Hyperspace PR Bot DL to get the latest announcements and pilot features!
PR Bot Information
Version:
1.20.11| 📖 Documentation | 🚨 Create Incident | 💬 Feedbackanthropic--claude-4.6-sonnetbc301571-3a96-4115-b847-53de15401511pull_request.opened