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

'basic link' button and inline with number in Coach #8720

Merged
merged 2 commits into from Nov 18, 2021

Conversation

abhimnc
Copy link
Contributor

@abhimnc abhimnc commented Nov 17, 2021

Summary

Fixes #8629

view learners' button in Coach has incorrect styling
Button is a 'basic link' and inline with number, not on next line
image

References

#8629

Reviewer guidance

is it okay? Do I need modify more?
image


Testing checklist

  • [-] Contributor has fully tested the PR manually
  • [-] If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • [-] PR has the correct target branch and milestone
  • [-] PR has 'needs review' or 'work-in-progress' ##label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@rtibbles
Copy link
Member

You have based your work here off the develop branch, which includes commits not in the release-v0.15.x branch.

To fix this, I'd recommend making sure your local copy of release-v0.15.x is up to date, and then do the following:

git checkout my-awesome-changes
git rebase -i release-v0.15.x

The second command here will open a text editor and prompt you with a list of commits. You should mark all the commits as 'drop' except your added alignment space commit.

Once that is done, you should force push to the remote my-awesome-changes branch, which will update this PR.

Copy link
Contributor

@indirectlylit indirectlylit left a comment

Choose a reason for hiding this comment

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

Thank you!

A couple small changes requested

package.json Outdated
Comment on lines 66 to 69
]
],
"dependencies": {
"caniuse-lite": "^1.0.30001280"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert the changes to package.json and yarn.lock as they are not directly related to this fix.

I've opened a follow-up issue related to the warnings you were seeing here: #8728

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not able to revert changes to package.json and yarn.lock by git commands. should i delete this branch and create a new one with applied changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi – I force-pushed an update to your branch which keeps only the relevant files.

For future reference, it's possible to break a previous commit in pieces and choose only portions of that commit using an interactive rebase.

See for example: https://stackoverflow.com/questions/6217156/break-a-previous-commit-into-multiple-commits

@indirectlylit indirectlylit added this to the 0.15.0 milestone Nov 18, 2021
Copy link
Contributor

@indirectlylit indirectlylit left a comment

Choose a reason for hiding this comment

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

looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'view learners' button in Coach has incorrect styling
3 participants