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.

Sign up for GitHub

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

Jump to bottom

Preview homepage #2265

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Preview homepage #2265

merged 2 commits into from
Jul 29, 2024

Conversation

syed-ali-tw
Copy link
Contributor

@syed-ali-tw syed-ali-tw commented Jul 26, 2024 •
edited
Loading

This commit involves changes to

  • Fix sidebar navigation to adjust for mobile devices, (this seems to be a miss from previous discussion).
  • Add preview functionality to view popular links on draft frontend and live frontend using 'preview' link and 'View on GOV.UK' link.

Trello

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

@syed-ali-tw syed-ali-tw force-pushed the preview-homepage branch 3 times, most recently from 7227c46 to 792fd9d Compare July 26, 2024 17:11
davidtrussler
davidtrussler approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@davidtrussler davidtrussler left a comment

Choose a reason for hiding this comment

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

LGTM 👍
(Mostly from Frontend perspective - probably should also get an approval from a more Rails perspective)

@syed-ali-tw syed-ali-tw force-pushed the preview-homepage branch 2 times, most recently from 758e409 to 41ea46b Compare July 29, 2024 10:47
baisa
baisa reviewed Jul 29, 2024
View reviewed changes
app/assets/stylesheets/popular_links.scss Outdated
border-top: 2px solid $govuk-brand-colour;
.sidebar-components {
border-top: 2px solid $govuk-brand-colour;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a new line here

baisa
baisa reviewed Jul 29, 2024
View reviewed changes
app/views/homepage/popular_links/_sidebar.html.erb Outdated
<% end %>
</div>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

New line here as well

baisa
baisa approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@baisa baisa left a comment

Choose a reason for hiding this comment

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

All looks good (just 2 tiny change requests) 👌

@syed-ali-tw syed-ali-tw merged commit cefa0bb into main Jul 29, 2024
12 checks passed
@syed-ali-tw syed-ali-tw deleted the preview-homepage branch July 29, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baisa baisa baisa approved these changes

@davidtrussler davidtrussler davidtrussler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@syed-ali-tw @baisa @davidtrussler