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

Fixes #37645 - Tab title missing in tableindexpage #10242

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

MariaAga
Copy link
Member

Helmet (from Head) sends its children directly to the head tag, so it doesnt make sense to send it text.
title html tag defines the title of the tab, and should only be used in the head.
The change in #9878 broke some table pages that now dont have a title, like hardware models, since it using header as headerText.

@MariaAga
Copy link
Member Author

MariaAga commented Aug 1, 2024

@jeremylenz / @parthaa could you take a look?

Copy link
Contributor

@kmalyjur kmalyjur left a comment

Choose a reason for hiding this comment

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

Works as it should and is unified 👍

@param {string}{headerText} - the header text for the page
@param {string}{header} - header node; default is <title>{headerText}</title>
@param {React.ReactNode}{customHeader} - a custom header to be rendered instead of the default header
@param {string}{headerText} - DEPRECATED - the header text for the page
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we delete the headerText?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

LGTM, works as expected

thanks @MariaAga !

@jeremylenz jeremylenz merged commit 75e33bc into theforeman:develop Aug 7, 2024
66 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants