feat(context-menu): implementa po-context-menu, adiciona activatedTab/focusTab ao po-tabs#2780
feat(context-menu): implementa po-context-menu, adiciona activatedTab/focusTab ao po-tabs#2780anderson-gregorio-totvs wants to merge 18 commits intomasterfrom
Conversation
- corrige effect que sobrescrevia sanitizacao (dead code) - adiciona output itemSelected para propagar selecao ao parent - corrige comparacao por referencia para comparacao por label - corrige handlerTooltip para atualizar via signal update - torna action opcional na interface PoContextMenuItem - simplifica sanitizeSelection com findIndex - adiciona tipo de retorno void ao handlerTooltip - remove import nao utilizado (tick) - adiciona PoTooltipModule ao TestBed - corrige typo handlerExpeanded e formato @default - remove chamada ngOnInit inexistente dos testes - usa setInput ao inves de items.set() nos testes - implementa testes unitarios abrangentes para cobertura completa Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| const label = value.firstElementChild as HTMLSpanElement; | ||
| if (label.scrollHeight > label.offsetHeight) { | ||
| this._items.update(items => items.map(i => (i === item ? { ...i, tooltip: item.label } : i))); |
There was a problem hiding this comment.
🟡 handlerTooltip silently fails to match item after selectItem due to reference equality on new spread objects
In handlerTooltip at line 72, the code uses reference equality (i === item) to find the matching item in _items. However, selectItem (line 52-55) replaces ALL items in _items with new spread copies ({ ...i, selected: ... }). While Angular's change detection typically re-renders the template before a new mouseenter event fires, this creates an inconsistency with selectItem which uses label-based equality (i.label === item.label) for the same purpose. If for any reason the template hasn't re-rendered yet (e.g., during the same microtask), the reference check will silently fail and no tooltip will be set. Using label-based matching (consistent with selectItem) would be more robust.
Inconsistency between selectItem and handlerTooltip matching strategies
selectItem at po-context-menu.component.ts:54 uses i.label === item.label (label equality), while handlerTooltip at po-context-menu.component.ts:72 uses i === item (reference equality). After selectItem creates entirely new objects via spread at lines 52-55, any stale reference from the template would fail the === check.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| @if (expanded()) { | ||
| <nav class="po-context-menu-body"> | ||
| <ul class="po-context-menu-list" role="menu"> | ||
| @for (item of _items(); track item.label) { |
There was a problem hiding this comment.
🟡 Using item.label as both @for track key and selection identifier breaks with duplicate labels
The template at po-context-menu.component.html:27 uses track item.label in the @for loop, and selectItem at po-context-menu.component.ts:59 matches items by i.label === item.label. If a consumer provides two items with the same label (e.g., [{label: 'Settings', action: fn1}, {label: 'Settings', action: fn2}]), two things break: (1) Angular's @for requires unique track values and will produce incorrect DOM diffing or runtime errors with duplicate keys, and (2) selectItem will mark ALL items sharing that label as selected: true, violating the single-selection invariant the component enforces elsewhere via sanitizeSelection.
Prompt for agents
In projects/ui/src/lib/components/po-context-menu/po-context-menu.component.html line 27, change `track item.label` to `track $index` (or add a unique `id` field to the PoContextMenuItem interface and use `track item.id`). Additionally, in projects/ui/src/lib/components/po-context-menu/po-context-menu.component.ts line 59 inside `selectItem`, change the matching from label-based (`i.label === item.label`) to index-based or identity-based matching to avoid selecting multiple items with the same label.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const inferTypeFromCode = codeLine => { | ||
| // Extrai tipo genérico | ||
| const genericMatch = codeLine.match(/input<([^>]+)>/); | ||
| const genericMatch = codeLine.match(/(?:input|output|model)<([^>]+)>/); |
There was a problem hiding this comment.
🟡 Greedy .+ in generic type regex truncates nested generic types like Array<PoContextMenuItem>
The regex /(?:input(?:\.required)?|output|model)<(.+)>(?=\s*\()/ at projects/portal/docs/processors/helpers/functions.js:125 uses greedy .+ to capture the generic type parameter. For nested generics, the greedy quantifier backtracks to find the > immediately before ( (due to the lookahead), which is the outermost closing > — causing the captured group to miss the inner closing >. This directly impacts the new PoContextMenuBaseComponent's items property defined as input<Array<PoContextMenuItem>>([], { alias: 'p-items' }): the type captured would be Array<PoContextMenuItem instead of Array<PoContextMenuItem>. The fix is to use non-greedy .+?, which correctly finds the first > that satisfies the (?=\s*\() lookahead.
Detailed trace for input<Array<PoContextMenuItem>>([])
With greedy .+:
.+=Array<PoContextMenuItem>→>matches second>→ lookahead sees(→ capture:Array<PoContextMenuItem❌
With non-greedy .+?:
.+?=Array<PoContextMenuItem→>matches first>→ lookahead sees>not(→ fail, continue.+?=Array<PoContextMenuItem>→>matches second>→ lookahead sees(→ capture:Array<PoContextMenuItem>✅
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 18ec322 — updated both regex patterns to use .+ with lookahead (?=\s*\() instead of [^>]+, so nested generics like Array<PoContextMenuItem> and string | TemplateRef<void> are captured correctly.
There was a problem hiding this comment.
The greedy .+ actually works correctly here. The lookahead (?=\s*\() constrains the match — the regex engine backtracks from the right until it finds a > followed by (, which is always the outermost closing >.\n\nTrace for input<Array<PoContextMenuItem>>([], ...):\n- .+ greedily matches everything, then backtracks\n- Finds > at position right before ( (the second >)\n- .+ = Array<PoContextMenuItem> → > matches second > → lookahead sees ( → capture: Array<PoContextMenuItem> ✅\n\nBoth greedy and non-greedy produce identical results for all signal patterns because the >(?=\s*\() anchor forces the correct match point. No change needed.
| property.isOptional = true; | ||
| } | ||
|
|
||
| if (property.type.includes('input') && property.Input !== undefined) { | ||
| const { type, alias } = inferTypeFromInput(property.type); | ||
| // Tratamento input e model no Signal | ||
| if ((property.type.includes('input') || property.type.includes('model'))) { | ||
| const { type, alias } = inferTypeFromCode(property.type); | ||
| properties[index].type = type; | ||
| properties[index].directiveInputAlias = alias; | ||
| properties[index].isDirectiveInput = true; | ||
| } |
There was a problem hiding this comment.
🟡 Docs processor does not classify model() signal properties as outputs
Angular's model() signal creates a two-way binding that acts as both an input and an output. In resolveProperties at projects/portal/docs/processors/helpers/functions.js:161-166, properties whose type includes model are only classified as inputs (isDirectiveInput = true), but isDirectiveOutput is never set. The output check at line 169 (property.type.includes('output')) does not match model. This means the expanded property in PoContextMenuBaseComponent (which uses model<boolean>) will not be documented as supporting two-way binding output events.
| property.isOptional = true; | |
| } | |
| if (property.type.includes('input') && property.Input !== undefined) { | |
| const { type, alias } = inferTypeFromInput(property.type); | |
| // Tratamento input e model no Signal | |
| if ((property.type.includes('input') || property.type.includes('model'))) { | |
| const { type, alias } = inferTypeFromCode(property.type); | |
| properties[index].type = type; | |
| properties[index].directiveInputAlias = alias; | |
| properties[index].isDirectiveInput = true; | |
| } | |
| // Tratamento input e model no Signal | |
| if ((property.type.includes('input') || property.type.includes('model'))) { | |
| const { type, alias } = inferTypeFromCode(property.type); | |
| properties[index].type = type; | |
| properties[index].directiveInputAlias = alias; | |
| properties[index].isDirectiveInput = true; | |
| } | |
| // model() also acts as an output for two-way binding | |
| if (property.type.includes('model')) { | |
| const { alias } = inferTypeFromCode(property.type); | |
| properties[index].directiveOutputAlias = alias ? alias + 'Change' : null; | |
| properties[index].isDirectiveOutput = true; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
projects/ui/src/lib/components/po-context-menu/po-context-menu-base.component.ts
Outdated
Show resolved
Hide resolved
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
bot
changed the title
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…a doc Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
anderson-gregorio-totvs
force-pushed
the
po-context-menu/devin
branch
from
2f7c815 to
2bec47e
Compare
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
feat(context-menu): implementa po-context-menu + melhorias no po-tabs
Summary
po-context-menu (new component)
Adds a new
po-context-menusidebar component for contextual module navigation, inspired bypo-menu. The component supports:p-expanded(viamodel())selected: truevia effect)p-item-selectedoutput event to notify parent on selectionpreventDefault) and ARIA attributesChangeDetectionStrategy.OnPushwith Angular Signals (input(),model(),signal(),output())New files: base component, component, template, interface, module, barrel index, spec, sample-basic, sample-labs
Modified files:
components.module.ts,components/index.ts,po-icon-dictionary.ts(addedICON_SIDEBAR,ICON_SIDEBAR_SIMPLES)po-tabs enhancements
activatedTaboutput onPoTabBaseComponent: newoutput<PoTabBaseComponent>({ alias: 'p-activated-tab' })signal emitted when a tab becomes activefocusTab(id)method onPoTabsComponent: programmatically activates a tab by itsid, with guard against non-matching IDs and JSDoc including@ViewChildusage exampleidproperty changed from plain optional property to@Input('id')to support external identificationpo-tab.base.component.spec.ts: replaced directnew PoTabComponent()withTestBed.runInInjectionContext()to support the newoutput()signalpo-tabs.component.spec.ts: addedactivatedTabmock toonTabActiveandonTabActiveByDropdowntestsPortal docs processor
inferTypeFromInput→inferTypeFromCodeto handleinput(),input.required(),output(), andmodel()signal patterns@Input/@OutputJSDoc tags fromconfiguration.js— processor now infers input/output status directly from codeoutput()are now correctly marked asisDirectiveOutputwithtype: 'EventEmitter'input()with no generic/value) no longer incorrectly inferred asnumbertypeUpdates since last revision
@ToDoplaceholders with full JSDoc foractivatedTaboutput andfocusTabmethod; fixed stray backtick and missing closing quote inpo-context-menu-base.component.tsNG0203: output() can only be used within an injection contextby wrapping component instantiation inTestBed.runInInjectionContext(); addedactivatedTabmocks to po-tabs test suitesample-po-context-menu-basicandsample-po-context-menu-basic-labsexamplesif (tab)check to prevent TypeError when no tab matches the given IDvalue !== ''check to prevent empty-string values from being incorrectly typed asnumber$event.preventDefault()to(keydown.space)in po-context-menu to prevent page scrollinput.required<T>()pattern viainput(?:\.required)?regex(p-expandChange)to(p-expandedChange)— Angular'smodel()with aliasp-expandedgenerates the output eventp-expandedChange(convention:<alias>Change)inferTypeFromCoderegex patterns from[^>]+to.+with lookahead(?=\s*\()so nested generic types likeArray<PoContextMenuItem>andstring | TemplateRef<void>are captured correctly instead of truncatedCI Status: All 8682 unit tests passing ✅
Review & Testing Checklist for Human
.+with lookahead(?=\s*\()to handle nested generics. While this should work for standard signal declarations, test doc generation with edge cases: multi-line declarations, unusual formatting/whitespace, deeply nested generics (e.g.,Map<string, Array<T>>), and union types with generics.processPropertyDocbehavior change — Previously, explicit@Input/@OutputJSDoc tags took absolute priority. Now, decorator-based detection runs first, with signal-based detection as fallback (||operator). If any component in the codebase used explicit JSDoc tags to override decorator detection, those docs may now be incorrect.idproperty changed to@Input('id')inPoTabBaseComponent(wasid?: string = uuid()). This makesidbindable from templates, which changes behavior — verify no existing consumers rely on the old non-input behavior and that Angular's own elementidattribute binding doesn't conflict.activatedTab.emit(tab)fires beforedeactivateTab()(line 219 ofpo-tabs.component.ts) — verify this ordering is intentional and doesn't cause issues with tab lifecycle or event listeners.focusTab(id)and verify(p-activated-tab)event fires with correctPoTabBaseComponentinstancefocusTabwith non-existent IDs (should silently do nothing, not crash)idto a po-tab from parent template and verify it's accessible viafocusTabinput(),input.required(),output(),model()) display correct types, especially nested genericsNotes
SamplePoContextMenuBasicComponentandSamplePoContextMenuBasicLabsComponentneed to be declared in a documentation module (e.g.,DocPoContextMenuModule) for portal build.index.tsdoesn't export the base class — add it if consumers need to extend.