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

[#10922] Added top padding to Add All Instructors button #10923

Conversation

srdeotarse
Copy link
Contributor

Fixes #10922

PR Checklist

Ensure that you have:

  • Read and understood our PR guideline: https://github.com/TEAMMATES/teammates/blob/master/docs/process.md#step-4-submit-a-pr

    • Added the issue number to the "Fixes" keyword above
    • Titled the PR as specified in the abovementioned document
  • Made your changes on a branch other than master and release

  • Gone through all the changes in this PR and ensured that:

    • They addressed one (and only one) issue
    • No unintended changes were made
  • Run and passed static analysis: ./gradlew lint and npm run lint

  • Added/updated tests, if changes in functionality were involved

  • Added/updated documentation to public APIs (classes, methods, variables), if applicable

Outline of Solution

Surrounded 'Add All Instructors' button with div containing class="top-padded" in /teammates/src/web/app/pages-admin/admin-home-page/admin-home-page.component.html

Changed top padding of 'Add All Instructors' button form this -
image
To this -
image

@srdeotarse srdeotarse closed this Jan 19, 2021
@srdeotarse srdeotarse reopened this Jan 19, 2021
@Derek-Hardy
Copy link
Contributor

There are failures on snapshot test suites because your solution has modified HTML elements in admin-home-page component.

Remember to update snapshots before submission. Commonly we do npm test followed by option u to update failed snapshots after modification.

If unfamiliar, you can refer to the documentation or Jest official website.

@Derek-Hardy Derek-Hardy added the s.Ongoing The PR is being worked on by the author(s) label Jan 19, 2021
@Derek-Hardy Derek-Hardy self-assigned this Jan 19, 2021
@srdeotarse
Copy link
Contributor Author

There are failures on snapshot test suites because your solution has modified HTML elements in admin-home-page component.

Remember to update snapshots before submission. Commonly we do npm test followed by option u to update failed snapshots after modification.

If unfamiliar, you can refer to the documentation or Jest official website.

I have removed div with 'top-padded' class, added class 'top-padded' to 'add-all-instructors' button and updated snapshot.

Copy link
Contributor

@Derek-Hardy Derek-Hardy left a comment

Choose a reason for hiding this comment

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

LGTM

@Derek-Hardy Derek-Hardy added s.FinalReview The PR is ready for final review and removed s.Ongoing The PR is being worked on by the author(s) labels Jan 20, 2021
@ChooJeremy ChooJeremy removed the request for review from madanalogy January 21, 2021 01:41
@ChooJeremy ChooJeremy added the s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging label Jan 21, 2021
@ChooJeremy ChooJeremy self-assigned this Jan 21, 2021
@ChooJeremy ChooJeremy removed the s.FinalReview The PR is ready for final review label Jan 21, 2021
@ChooJeremy ChooJeremy merged commit 93f990e into TEAMMATES:master Jan 21, 2021
@madanalogy madanalogy added the c.Bug Bug/defect report label Jan 21, 2021
@madanalogy madanalogy added this to the V7.9.0 milestone Jan 21, 2021
@srdeotarse srdeotarse deleted the 10922-add-top-padding-to-add-all-instructors-button branch January 23, 2021 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Bug Bug/defect report s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No top padding for 'Add All Instructors' button
4 participants