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

Fixed missing frame lines in ReadCode #1759

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

ranbena
Copy link
Contributor

@ranbena ranbena commented Dec 19, 2018

What current issue(s) from Trello/Github does this address?
Noticed that in the sketch design file there should be markers in the send modal QR scan rubric AND that there are related DOM elements in the code, but the markers do not appear.

Before After
screen shot 2018-12-19 at 15 12 18 screen shot 2018-12-19 at 15 11 25

What problem does this PR solve?

  1. The existing DOM element classnames were in plain text instead of CSS module properties.
  2. The design implementation wasn't totally accurate - the marker edges should be pointy.

screen shot 2018-12-19 at 14 59 56

How did you solve this problem?

  1. Fixed classnames.
  2. Implemented pointy edged with the css border transparent trick.

How did you make sure your solution works?
Manually gazed at it like a madman.

Are there any special changes in the code that we should be aware of?
Since Sass @extend is already used for this, I kicked it up a notch making it into placeholder classes (.frameLine -> %frameLine) which is better for perf in this case.

It all comes down to this. Ain't it nice? 😬

.frameLine {
  &TopLeft {
    @extend %frameLine, %frameLineTop, %frameLineLeft;
  }
  &TopRight {
    @extend %frameLine, %frameLineTop, %frameLineRight;
  }
  &BottomLeft {
    @extend %frameLine, %frameLineBottom, %frameLineLeft;
  }
  &BottomRight {
    @extend %frameLine, %frameLineBottom, %frameLineRight;
  }
}

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5458

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 41.087%

Totals Coverage Status
Change from base Build 5454: 0.0%
Covered Lines: 1025
Relevant Lines: 2285

💛 - Coveralls

Copy link
Contributor

@evgenyboxer evgenyboxer left a comment

Choose a reason for hiding this comment

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

Very nice!!

@evgenyboxer evgenyboxer merged commit 2fffdf2 into CityOfZion:dev Dec 21, 2018
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.

4 participants