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

Center align icon of send box button #3673

Merged
merged 3 commits into from
Jan 19, 2021

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Jan 17, 2021

Fixes #3672.

Changelog Entry

Fixed

  • Fixes #3672. Center the icon of send box buttons vertically and horizontally, by @compulim in PR #3673

Description

The icon is not vertically centered. It is off by 1 pixel.

Design

Specific Changes

  • All VRT snapshots are updated
  • Some snapshots are deleted because they are orphans
  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

TBH I haven't gone through all tests and I'm trusting based off the code changes that test img updates are all valid haha. LGTM

@compulim
Copy link
Contributor Author

Going through 415 images aren't quick without good tools. And I found a few bugs in our existing snapshots due to retries.

I mean, if we run a test, it failed, we will retry. The snapshot counter will continue to cumulate.

That means, snapshot abc-1-snap.png may fail. It trigger a rerun and will compare against to abc-2-snap.png. So, some snapshot of *-1-snap.png may be "always failing".

Overall test is still verifying our work. But due to this, we lost 1 retry chance as a result because *-1-snap.png is always failing.

Anyway, will look into this issue in the future.

p.s. I also removed all orphans I believe.

@compulim compulim merged commit ca0ad75 into microsoft:master Jan 19, 2021
@compulim compulim deleted the feat-align-icon branch January 19, 2021 21:40
@compulim compulim mentioned this pull request Mar 2, 2021
52 tasks
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.

Icons inside send box buttons are not vertically centered
2 participants