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

fix #15144 DynamicDialog get closeAriaLabel #15152

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

TaneliTuomola
Copy link

fixes #15144

Copy link

vercel bot commented Mar 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primeng ⬜️ Ignored (Inspect) Visit Preview Apr 2, 2024 8:35am

@cetincakiroglu
Copy link
Contributor

cetincakiroglu commented Mar 29, 2024

Hi @TaneliTuomola,

For aria labels, you should place it inside the aria object in translation object in primengconfig.ts (don't forget to update interfaces). Also, you should access it inside the component below to make it translatable. And one more thing, close exists in aria object inside the translation in primengconfig.ts. Could you please useclose instead of creating a new property for the same thing?

Correct usage:

    get closeAriaLabel(): string {
        return this.config.getTranslation(TranslationKeys.ARIA)['close'];
    }

You can find examples of this usage in various places inside the library.
I'll accept your PR after these changes are done. Please tag me after the changes.

Thanks in advance.

@cetincakiroglu cetincakiroglu added the Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. label Mar 29, 2024
@SoyDiego
Copy link
Contributor

SoyDiego commented Apr 2, 2024

Hi @TaneliTuomola,

For aria labels, you should place it inside the aria object in translation object in primengconfig.ts (don't forget to update interfaces). Also, you should access it inside the component below to make it translatable. And one more thing, close exists in aria object inside the translation in primengconfig.ts. Could you please useclose instead of creating a new property for the same thing?

Correct usage:

    get closeAriaLabel(): string {
        return this.config.getTranslation(TranslationKeys.ARIA)['close'];
    }

You can find examples of this usage in various places inside the library. I'll accept your PR after these changes are done. Please tag me after the changes.

Thanks in advance.

good catch!

@TaneliTuomola
Copy link
Author

TaneliTuomola commented Apr 2, 2024

Hi @TaneliTuomola,

For aria labels, you should place it inside the aria object in translation object in primengconfig.ts (don't forget to update interfaces). Also, you should access it inside the component below to make it translatable. And one more thing, close exists in aria object inside the translation in primengconfig.ts. Could you please useclose instead of creating a new property for the same thing?

Correct usage:

    get closeAriaLabel(): string {
        return this.config.getTranslation(TranslationKeys.ARIA)['close'];
    }

You can find examples of this usage in various places inside the library. I'll accept your PR after these changes are done. Please tag me after the changes.

Thanks in advance.

Thanks for the advice @cetincakiroglu. Global text is better for setting the text of the Close button than a separate property.

Would it be worth refactoring the icon button texts of the Dialog and ConfirmDialog components in the same way? Now they use a separate Input property: @Input() closeAriaLabel: string | undefined;

@cetincakiroglu cetincakiroglu removed the Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. label Apr 9, 2024
@cetincakiroglu cetincakiroglu merged commit 8beb37c into primefaces:master Apr 9, 2024
2 of 3 checks passed
@cetincakiroglu
Copy link
Contributor

Hi @TaneliTuomola,

Thanks a lot for the effort and support!

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.

DynamicDialog: accessibility- CloseAriaLabel not working
3 participants