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

Fix carousel alignment and test perf improvement #1641

Merged
merged 10 commits into from
Jan 28, 2019

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Jan 25, 2019

Fix #1635.

Background

Alignment was broken due to #1560 and #1561 when showing carousel with bot avatar initials.

This PR will also add tests for carousel and stacked layout, and a bit more stuff:

  • Test perf improvement for suggested actions tests
    • Replaced sleep with conditions (a.k.a. proactive retry)
    • Updated snapshots with cursor removed
  • Added first page object: hideCursor
    • Will evaluate how effective this coding pattern is, and add more later
  • Added timeout settings via constants.json

All tests running locally on my box took 49 seconds.

Changelog

Fixed

@coveralls
Copy link

coveralls commented Jan 25, 2019

Pull Request Test Coverage Report for Build 869

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.6%) to 52.495%

Totals Coverage Status
Change from base Build 862: 3.6%
Covered Lines: 879
Relevant Lines: 1522

💛 - Coveralls

@corinagum
Copy link
Contributor

Partial fix for #1625

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.

Let's get your two pending PR's in before we worry about this one.

CHANGELOG.md Outdated Show resolved Hide resolved
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.

Pending a question and minor CHANGELOG.md fix :)

CHANGELOG.md Outdated Show resolved Hide resolved
__tests__/basic.js Show resolved Hide resolved
@compulim compulim merged commit af98009 into microsoft:master Jan 28, 2019
@compulim compulim deleted the fix-1635 branch January 28, 2019 19:20
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.

3 participants