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

Add more fields to BlogPost card #1935

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

Conversation

FadhlanR
Copy link
Contributor

Screenshot 2024-12-13 at 16 12 26

@FadhlanR FadhlanR force-pushed the cs-7541-add-more-fields-to-blogpost branch from 0df29c2 to fd4ba09 Compare December 13, 2024 09:32
Copy link

github-actions bot commented Dec 13, 2024

Host Test Results

    1 files  ±0      1 suites  ±0   20m 15s ⏱️ +19s
710 tests ±0  708 ✔️ ±0  2 💤 ±0  0 ±0 
715 runs  ±0  713 ✔️ ±0  2 💤 ±0  0 ±0 

Results for commit 6e78781. ± Comparison against base commit c4ffc5e.

♻️ This comment has been updated with latest results.

@FadhlanR FadhlanR marked this pull request as ready for review December 13, 2024 10:15
@jurgenwerk jurgenwerk requested a review from a team December 13, 2024 10:19
@@ -607,7 +637,15 @@ export class BlogPost extends CardDef {
<li class='pub-date'>
Published on
<time timestamp={{@model.datePublishedIsoTimestamp}}>
{{this.formattedDatePublished}}
{{@model.formattedDatePublished}}
Copy link
Contributor

@tintinthong tintinthong Dec 13, 2024

Choose a reason for hiding this comment

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

I've never seen this pattern before and I don't know why I always assumed this to be wrong.

You use a getter at the root of the CardDef and you use @model.formattedDatePublished to access the model value. This is great bcos you can share these across formats. But, is this different than a computed or not -- do you know? Typically, all my getters exist on the format template.

In general, this is not a change request. Just a curiosity. I thought perhaps the types will fail or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm unsure if this is the correct pattern or not. However, it works, and I notice the same pattern in a few cards, such as catalog-entry and leaflet-gtfs. Are you referring to a computed field? This is different from a computed field.

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