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

feat: move components to angular-accelerator #526

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

mollend0
Copy link

@mollend0 mollend0 commented Oct 4, 2024

No description provided.

@@ -0,0 +1,3 @@
<div [ocxContent]="title" [style]="styleClass">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The styleClass input should be applied as a class, not as a custom style. The same also applies to the content container component. Please also make sure that the style classes provided via input have a higher priority than the classes which are added by the directive itself. This way we don't only allow devs to specify additional classes but also allow them to overwrite existing style classes such as padding through the given input.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please verify the class merging behavior described above in a unit test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After clarifying with @SchettlerKoehler we decided to delete all files from PIA after migrating them to angular-accelerator. Importing the migrated components from PIA should still work because PIA re-exports everything from angular-accelerator (see attached file).

export * from '@onecx/angular-accelerator'

Please delete all components and files that have been migrated as part of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file as it didn't exist previously and shouldn't be necessary in our repo.

@bastianjakobi
Copy link
Contributor

Please also update your fork's main branch and merge it into your working branch

{{ item?.columnName ? (item.columnName | translate) : ''}}
</ng-template></p-dropdown
>
<label for="dataListGridSortingDropdown">{{ ("OCX_LIST_GRID_SORT.DROPDOWN.ARIA_LABEL" | translate) }}</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please create a separate translation OCX_LIST_GRID_SORT.DROPDOWN.LABEL for the floating label? This way, we can change the aria label in the future without unexpectedly modifying the displayed floating label. The new translation key can have the exact same value as the previously used aria label translation.

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.

2 participants