Skip to content

Conversation

@wagnerlmichael
Copy link
Member

@wagnerlmichael wagnerlmichael commented Nov 6, 2025

Edit the language from 'top 5 significant sale' to 'significant sales'. The top 5 language could be confusing as it could be interpreted as referring to market value.

Dev PIN: 01011000040000

Closes part of #147

@wagnerlmichael wagnerlmichael linked an issue Nov 6, 2025 that may be closed by this pull request
2 tasks
HOMEVAL_ASSESSMENT_CARD_TABLE = "pinval.vw_assessment_card"
HOMEVAL_COMP_TABLE = "pinval.vw_comp"
HOMEVAL_DATA_DICT_TABLE = "pinval.vars_dict"
HOMEVAL_DATA_DICT_TABLE = "z_ci_update_vars_dict_bldg_sqft_desc_pinval.vars_dict"
Copy link
Member Author

Choose a reason for hiding this comment

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

Reminder: Revert before merging

@wagnerlmichael wagnerlmichael changed the title [WIP] Text tweaks from public feedback Text tweaks from public feedback Nov 18, 2025
@wagnerlmichael wagnerlmichael marked this pull request as ready for review November 18, 2025 20:32
Copy link
Member

@jeancochrane jeancochrane left a comment

Choose a reason for hiding this comment

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

Looking good! Unfortunately, I am yet again second guessing the exact phrasing of these headers... tagging in @ccao-jardine for feedback on my feedback as well. I don't think it's a huge deal, and I don't necessarily think we need to redeploy the reports even if we do change these two headers, because I could easily just tweak the HTML in a live report to see what it looks like.


<!-- Comp map -->
<h2 class="mb-3">Top 5 Most Significant Sales</h2>
<h2 class="mb-3">The Most Significant Sales</h2>
Copy link
Member

Choose a reason for hiding this comment

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

[Nitpick, optional] Sorry for changing my mind yet again, but looking at it again today, this reads a bit weird to me -- kind of vague and mysterious, sort of like a book title more than a section header. Adding slightly more context makes it sound better to me, but maybe I'm overthinking it. What do you think?

Suggested change
<h2 class="mb-3">The Most Significant Sales</h2>
<h2 class="mb-3">The Most Significant Sales for Your Home</h2>
Image

Copy link
Member

Choose a reason for hiding this comment

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

I concur -- while our work can be mysterious and important, HomeVal should be clear and accessible. Thanks for flagging this! I do think that it is an improvement over "Top 5", but I'll join your overthinking.

Hmm. Now that I've reviewed this in context, I'm reminded that this header is also the first part of the report that even mentions significant sales, outside of the expandable "Click or tap to read more about this report" accordion (which we shouldn't assume people will read through).

"The Most Significant Sales for Your Home" feels a bit awkward, like we should either mean "Sales of my home" (not the correct description of what is to follow) or sales that are important to a home (homes probably don't have feelings about sales).

Thoughts on "The Most Significant Sales Used to Estimate Your Home's Value"? It's appreciably longer, which I don't like, but it doesn't look as bad as I would have thought on mobile (below width: 360 pixels). It still feels a little clunky to me.

image

Copy link
Member

Choose a reason for hiding this comment

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

@jeancochrane, @wagnerlmichael: Let's revert. Let's keep "Top 5 Most Significant Sales" until we have substantial evidence from multiple users that it's confusing enough to change. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Good by me!

Copy link
Member Author

Choose a reason for hiding this comment

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

Works for me! in that case, should we close this PR, and then merge ccao-data/data-architecture#926? @jeancochrane

Copy link
Member

Choose a reason for hiding this comment

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

Yup, that works for me @wagnerlmichael 👍🏻

id="sale-characteristics-{{ .card.card_num }}"
>
Characteristics for Top 5 Most Significant Sales
Characteristics for the Most Significant Sales
Copy link
Member

Choose a reason for hiding this comment

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

[Nitpick, optional] Similar to above, I'm feeling like "for" is not quite the right preposition here. "Of" sounds more natural to me, and matches the way we phrase the "summary" header below. Does that sound better to you too?

Suggested change
Characteristics for the Most Significant Sales
Characteristics of the Most Significant Sales
Image

Copy link
Member

Choose a reason for hiding this comment

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

Seconded, nice catch!

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.

Text tweaks from public feedback

3 participants