Improved Explain with AI button #2528
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| cursor: "pointer", // Pointer cursor to indicate it's interactable | ||
| }} | ||
| /> | ||
| ✨ |
There was a problem hiding this comment.
Replacing a styled icon component with a plain emoji may impact accessibility and visual consistency. Consider either retaining the styled icon or wrapping the emoji with appropriate accessibility attributes such as an aria-label.
| ✨ | |
| <InfoCircleOutlined | |
| style={{ | |
| fontSize: 16, | |
| color: style.colors.DARK_GRAY, | |
| }} | |
| aria-label="Information icon" | |
| /> |
There was a problem hiding this comment.
issue, blocking: I agree with the AI here. Encoding an emoji directly into a webpage is not really best practice. I also find this icon to take away from our brand identity. Opportunities for discovery and exploration certainly typify us, but I find this icon to be more "whimsical," which I think takes away from the seriousness we try to employ around our work.
@SEEDART007 could you look through https://ant.design/components/icon and see if anything would be better suited here? Or propose an alternative design? Feel free to just share a visual.
There was a problem hiding this comment.
let's also use the icon only once
There was a problem hiding this comment.
issue, blocking: I agree with the AI here. Encoding an emoji directly into a webpage is not really best practice. I also find this icon to take away from our brand identity. Opportunities for discovery and exploration certainly typify us, but I find this icon to be more "whimsical," which I think takes away from the seriousness we try to employ around our work.
@SEEDART007 could you look through https://ant.design/components/icon and see if anything would be better suited here? Or propose an alternative design? Feel free to just share a visual.
This is pretty good...what do you think??
There was a problem hiding this comment.
Is that from ant? We want one without text
There was a problem hiding this comment.
Ant design doesn't have anything based on ai much. this is flaticon which is better. can you choose and tell me which one should I use and also tell me why the lint test is failing??? @MaxGhenis @anth-volk
There was a problem hiding this comment.
Confirmed, ant lacks an AI icon like sparkles.
@anth-volk - per our recent CC meeting, let's drop the AI iconography here and just having it be a standard i tooltip with an "Explain" button. We can add ✨ Generated with AI inside the modal instead.
SEEDART007
changed the title
|
@MaxGhenis hey can you check it out!!is everything fine in it? |
github-project-automation
Bot
moved this from Todo
to PR: Review Requested
in policyengine-app
|
Sounds good. In that case @SEEDART007 could you revert to original styling
and make the i button more intuitive, per the original issue?
…On Sat, May 17, 2025, 11:17 Max Ghenis ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/pages/household/output/NetIncomeBreakdown.jsx
<#2528 (comment)>
:
> @@ -319,14 +319,7 @@ function VariableArithmetic(props) {
}}
className="info-icon-wrapper"
>
- <InfoCircleOutlined
- style={{
- fontSize: 14,
- marginLeft: 10,
- color: style.colors.DARK_GRAY,
- cursor: "pointer", // Pointer cursor to indicate it's interactable
- }}
- />
+ ✨
Confirmed, ant lacks an AI icon like sparkles.
@anth-volk <https://github.com/anth-volk> - per our recent CC meeting,
let's drop the AI iconography here and just having it be a standard i
tooltip with an "Explain" button. We can add ✨ Generated with AI inside
the modal instead.
—
Reply to this email directly, view it on GitHub
<#2528 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADSK7W4MYJKBLCYBK7PIV2T265HIZAVCNFSM6AAAAAB5CM7RYWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBYGMYTIOJUGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
u mean to use the old star icon.? but tell me why the lint test is failing? @anth-volk |
|
See Max's comment above and the original issue description - basically,
please keep the AI hover label the same, but make the i button clickable,
if I'm remembering correctly.
And did you run `make format` as described in the README?
On Sat, May 17, 2025, 13:55 Siddhartha Chakraborty ***@***.***>
wrote:
… *SEEDART007* left a comment (PolicyEngine/policyengine-app#2528)
<#2528 (comment)>
Sounds good. In that case @SEEDART007 <https://github.com/SEEDART007>
could you revert to original styling and make the i button more intuitive,
per the original issue?
… <#m_-3950319426812210838_>
On Sat, May 17, 2025, 11:17 Max Ghenis *@*.*> wrote: @.** commented on
this pull request. ------------------------------ In
src/pages/household/output/NetIncomeBreakdown.jsx <#2528 (comment)
<#2528 (comment)>>
: > @@ -319,14 +319,7 @@ function VariableArithmetic(props) { }}
className="info-icon-wrapper" > - <InfoCircleOutlined - style={{ -
fontSize: 14, - marginLeft: 10, - color: style.colors.DARK_GRAY, - cursor:
"pointer", // Pointer cursor to indicate it's interactable - }} - /> + ✨
Confirmed, ant lacks an AI icon like sparkles. @anth-volk
<https://github.com/anth-volk> https://github.com/anth-volk - per our
recent CC meeting, let's drop the AI iconography here and just having it be
a standard i tooltip with an "Explain" button. We can add ✨ Generated with
AI inside the modal instead. — Reply to this email directly, view it on
GitHub <#2528 (comment)
<#2528 (comment)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADSK7W4MYJKBLCYBK7PIV2T265HIZAVCNFSM6AAAAAB5CM7RYWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBYGMYTIOJUGA
. You are receiving this because you were mentioned.Message ID: *@*.***>
u mean to use the old star icon.? but tell me why the lint test is
failing? @anth-volk <https://github.com/anth-volk>
—
Reply to this email directly, view it on GitHub
<#2528 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADSK7W6KGYGHLTPQ7JTUJWL265ZYLAVCNFSM6AAAAAB5CM7RYWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBYGUYTGOJYGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Closing in favor of #2546. |
github-project-automation
Bot
moved this from PR: Review Requested
to Closed
in policyengine-app
#2301 Improved Explain with AI button
Screen.Recording.2025-05-14.113904.mp4