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

Simplified apply for your visa to train to teach #3958

Merged
merged 16 commits into from
May 21, 2024
Merged

Simplified apply for your visa to train to teach #3958

merged 16 commits into from
May 21, 2024

Conversation

laurahermionetennant92
Copy link
Contributor

@laurahermionetennant92 laurahermionetennant92 commented Apr 29, 2024 •
edited
Loading

Simplified apply for your visa to train to teach

Trello card

Context

Changes proposed in this pull request

Teacher trainee visa page cut back and simplified, with steps to follow to apply

Guidance to review

@laurahermionetennant92
Simplified apply for your visa to train to teach
d5ff86a 
Simplified apply for your visa to train to teach
@github-actions github-actions bot added the content Content changes label Apr 29, 2024
@laurahermionetennant92
Tweaks
2610805 
Tweaks
@laurahermionetennant92
Edits for greater clarity
224e5e1 
Edits for greater clarity
@laurahermionetennant92
Tweaks
df640f8 
Tweaks
@laurahermionetennant92
Copy link
Contributor Author

Hi Emma, please review when you get a sec. Thank you! Laura

emmaachesongray
emmaachesongray reviewed May 14, 2024
View reviewed changes
Copy link
Contributor

@emmaachesongray emmaachesongray left a comment

Choose a reason for hiding this comment

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

Hi Laura,

I've left some general comments and a few formatting issues I've seen from the demo review app. Let me know if you'd like to have a chat through.

app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md Outdated

You will not need a visa to train to teach in the UK if you:
### Student visa
Copy link
Contributor

Choose a reason for hiding this comment

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

This heading isn't aligned properly on the demo app - it looks like there's an extra space before the H3?

app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md Outdated

If you are currently studying in the UK on a student visa, you may be able to apply for a [graduate visa](#graduate-visa).
### Skilled worker visa
Copy link
Contributor

Choose a reason for hiding this comment

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

This heading isn't aligned properly on the demo app - it looks like there's an extra space before the H3?

Also, small point so feel free to ignore - but should this be 'Student Worker visa' to align with GOV.UK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skilled Worker? Yes good spot. Not sure why it is not aligning as no space, @martyn-w do you know why this might be happening?

app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md Outdated
@@ -36,208 +36,122 @@ keywords:

---

If you’re applying for teacher training as a non-UK citizen, you will need a visa or immigration status allowing you to study or work in England.
If you’re applying for teacher training as a non-UK citizen, you will need a [visa or immigration status allowing you to study or work in England](#check-your-status).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this text may get lost.

For me it seems more logical to have the 'check your status' content as the first H2. Or to move this content under the 'How to apply for your visa' heading?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I don't know if it's style to have some content under the yellow h1 before using an h2?
On the order, the vast majority of people will fall into the Student visa bucket so I want to prioritise that content.

@laurahermionetennant92
Emma tweaks
b410d33 
Emma tweaks
@laurahermionetennant92
Emma tweaks
3ccd9b0 
Emma tweaks
emmaachesongray
emmaachesongray reviewed May 15, 2024
View reviewed changes
Copy link
Contributor

@emmaachesongray emmaachesongray left a comment

Choose a reason for hiding this comment

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

Spotted a few more lower case 'Skilled worker' so have tweaked those. Then all good :) Is Martyn sorting out the alignment of the headers issue?

@emmaachesongray emmaachesongray marked this pull request as ready for review May 15, 2024 13:09
laurahermionetennant92 and others added 5 commits May 15, 2024 14:31
@laurahermionetennant92 @emmaachesongray
Update app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md
49c5b32 
Co-authored-by: emmaachesongray <168454683+emmaachesongray@users.noreply.github.com>
@laurahermionetennant92 @emmaachesongray
Update app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md
66ccb50 
Co-authored-by: emmaachesongray <168454683+emmaachesongray@users.noreply.github.com>
@laurahermionetennant92 @emmaachesongray
Update app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md
bbfb1d3 
Co-authored-by: emmaachesongray <168454683+emmaachesongray@users.noreply.github.com>
@laurahermionetennant92 @emmaachesongray
Update app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md
cd30af6 
Co-authored-by: emmaachesongray <168454683+emmaachesongray@users.noreply.github.com>
@laurahermionetennant92 @emmaachesongray
Update app/views/content/non-uk-teachers/visas-for-non-uk-trainees.md
76e38ad 
Co-authored-by: emmaachesongray <168454683+emmaachesongray@users.noreply.github.com>
@laurahermionetennant92
Copy link
Contributor Author

@emmaachesongray Martyn has fixed alignments – can you take another look so I can merge?

emmaachesongray
emmaachesongray previously approved these changes May 20, 2024
View reviewed changes
Copy link
Contributor

@emmaachesongray emmaachesongray left a comment

Choose a reason for hiding this comment

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

Look great to me :)

@laurahermionetennant92
Train to teach page visa section cut back
4ae0292 
Train to teach page visa section cut back
@github-actions GitHub Actions
Copy link

Review app deployed to https://get-into-teaching-app-review-3958.test.teacherservices.cloud

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
9 New issues
10 Accepted issues

Measures
0 Security Hotspots
96.3% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarCloud

@laurahermionetennant92
Copy link
Contributor Author

laurahermionetennant92 commented May 21, 2024 •
edited
Loading

hi sorry me again @emmaachesongray – I realised I could dramatically cut back the Train to teach page too – please could you have one more look?
See content under Apply for your visa

@laurahermionetennant92 laurahermionetennant92 merged commit 3507a43 into master May 21, 2024
26 checks passed
@laurahermionetennant92 laurahermionetennant92 deleted the Apply-for-your-visa-to-train-to-teach-simplified branch May 21, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emmaachesongray emmaachesongray emmaachesongray approved these changes

Assignees
No one assigned
Labels
content Content changes Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@laurahermionetennant92 @emmaachesongray @martyn-w