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

Refactor: agency index #430

Merged
merged 5 commits into from
Apr 14, 2022
Merged

Refactor: agency index #430

merged 5 commits into from
Apr 14, 2022

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Apr 14, 2022

Closes #367

Follows approach outlined in the ticket. Using a new template specific to agency_index gave us the flexibility to use blocktranslate for embedding the link to "Learn more about Cal-ITP Benefits."

Note that I didn't attempt to style the page. I think @machikoyasuda has an idea for doing a focused cleanup of styles in a separate ticket. Let me know though if that's not correct! 🙏

Latest screen from Figma mockup:
image

Current view on this branch:
image

Here's the Before (from dev branch):
image

UI tests pass

image

@angela-tran angela-tran added this to the Login.gov SSO milestone Apr 14, 2022
@angela-tran angela-tran requested a review from a team as a code owner April 14, 2022 00:51
@github-actions github-actions bot added the deployment-dev [auto] Changes that will trigger a deploy if merged to dev label Apr 14, 2022
@angela-tran angela-tran changed the title Refactor/agency index Refactor: agency index Apr 14, 2022
@machikoyasuda machikoyasuda self-requested a review April 14, 2022 01:25
Copy link
Member

@machikoyasuda machikoyasuda left a comment

Choose a reason for hiding this comment

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

LGTM! After merging this PR, need to make a ticket to:

  1. Get all new Spanish translations and add them
  2. Finalize CSS for all pages once all the copy and new pages/elements are in.

@angela-tran
Copy link
Member Author

I'll wait until tomorrow to merge this, in case anyone else wants to review

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Nice!

@angela-tran
Copy link
Member Author

LGTM! After merging this PR, need to make a ticket to:

1. Get all new Spanish translations and add them

2. Finalize CSS for all pages _once_ all the copy and new pages/elements are in.

Thanks for this list @machikoyasuda ! I think #359 captures that we're waiting to get finalized Spanish translations. I wouldn't be opposed to also having our own dev-centric ticket for actually adding those Spanish translations in if you think that would be good to have.

Do you want to create the GitHub issue for finalizing the CSS once all the pages/elements are in?

@machikoyasuda
Copy link
Member

@angela-tran Sounds good! Could you create that CSS ticket after you merge this PR?? Thank you 🙏

@angela-tran angela-tran merged commit 2788a8d into dev Apr 14, 2022
@angela-tran angela-tran deleted the refactor/agency-index branch April 14, 2022 17:10
@thekaveman
Copy link
Member

@angela-tran when you create that new ticket, can you please add it to the list on #363 too?

@angela-tran
Copy link
Member Author

angela-tran commented Apr 14, 2022

@angela-tran when you create that new ticket, can you please add it to the list on #363 too?

Yep, will do 👍

Added #435 to our epic #363

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment-dev [auto] Changes that will trigger a deploy if merged to dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update rendering of Agency index page
3 participants