Migrate Learn:AI/ML page #242
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Please add "Fixes #" to the issue description and remove "Feat/142" from the PR name |
There was a problem hiding this comment.
Hi @AnushkaNilangekar, thanks for your work on this! With the route added, I was able to see how the page looks in the deployment, so thanks for that!
The page looks pretty good and is mostly ready to go. Only problem is with the alignment of the AI/ML Resources, Our AI Journey and the Watch our AI Demo components on the page.
These appear to be either sticking to the left edge of the page or stretched out to both edges. I wonder if wrapping the contents in a container would fix the issue.
Also, there have been some major updates to the codebase over the past couple of weeks. Could you please rebase on the latest changes from main and update the PR if needed (probably the only change would be using the static layout in the router)?
AnushkaNilangekar
changed the title
AnushkaNilangekar
force-pushed
the
feat/142-migrate-ai-ml-page-to-app-v2
branch
from
1ad0126 to
6cf3ffa
Compare
|
Hi @SakshiKekre, thanks for all the comments! I've rebased with main and made updates to the layout. There are still a few inconsistencies across components, like spacing, page-specific colors, and header typography, that could use some standardization based on what we discussed earlier. I'm happy to update this page and/or adjust shared components if that helps keep things consistent. |
anth-volk
left a comment
anth-volk
left a comment
There was a problem hiding this comment.
Thanks for this @AnushkaNilangekar. I've left some comments. In a couple places, I asked why you're modifying shared components, but am now realizing we do actually need to modify a couple of them.
| direction={{ base: 'column', md: 'row' }} | ||
| align={{ base: 'stretch', md: 'center' }} | ||
| gap={{ base: 'md', md: 'xl' }} | ||
| <Box |
There was a problem hiding this comment.
question: Is this a change you're making now, or did you incorporate changes from a rebase and they're being improperly displayed as part of your PR?
second question: If you are making this change, why? This would modify all pages using this component.
| </Grid> | ||
| </Paper> | ||
| <Paper bg={getBackgroundColor()} radius={spacing.radius.lg} style={{ minHeight: '400px' }}> | ||
| <Title |
There was a problem hiding this comment.
question: Here, too, this impacts others' pages; what's the reasoning behind it?
| gap: spacing['3xl'], | ||
| }} | ||
| > | ||
| <CalloutWithImage |
There was a problem hiding this comment.
issue: Shoot, CalloutWithImage actually does have something we'd want to change app-wide. As part of this PR, could you modify the button to look the same as (but have larger font size than) the button we have in the calculator pages to create a new report? It has rounded edges, unlike this one, and is a darker green.
| </Paper> | ||
| <Paper bg={getBackgroundColor()} radius={spacing.radius.lg} style={{ minHeight: '400px' }}> | ||
| <Title | ||
| order={2} |
There was a problem hiding this comment.
issue: Having just said that, please increase the size of this title
| </> | ||
| ); | ||
|
|
||
| const rightColumnContent = ( |
There was a problem hiding this comment.
issue: Please round the edges on the image
Fixes #142
Description
Added a new AI & ML info page that explains PolicyEngine’s AI work, shows resource cards and a timeline, includes a how-it-works two-column section with an image, and embeds a demo video. The page reuses existing components and image assets.
Changes
Note:
The button for the "Making Policy Accessible With AI" component is not linked to a page yet. Happy to make changes as needed!