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

NEW: Empty link title fallbacks to Page. #37

Merged
merged 2 commits into from
Nov 11, 2021
Merged

Conversation

mfendeksilverstripe
Copy link
Collaborator

NEW: Empty link title fallbacks to Page

Fixes #34

}

// Use page title as a default value in case CMS user didn't provide the title
$this->Title = $this->Page->Title;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mfendeksilverstripe What would be your thoughts on us implementing a getTitle() method instead? This way, if the Page:$Title updated, we could have the Link title update as well.

The Page would already be loaded in memory in order to determine the Link, so I don't think there would be any performance impact?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think more extensibility here is a good idea 👍

@mfendeksilverstripe
Copy link
Collaborator Author

@chrispenny I've added new changes based on your feedback, please review.

@chrispenny chrispenny merged commit 3e0f699 into 1 Nov 11, 2021
@chrispenny chrispenny deleted the feature/title-fallback branch November 11, 2021 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants