Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Fixed missed icons and styles referencing the old icons #1987

Merged
merged 11 commits into from
Sep 19, 2018

Conversation

Blackbaud-TrevorBurch
Copy link
Member

Resolves #1983

Copy link
Contributor

@blackbaud-conorwright blackbaud-conorwright left a comment

Choose a reason for hiding this comment

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

Just a couple comments and minor concerns

<i
class="fa fa-lg"
[ngClass]="['fa-' + buttonType]"></i>
<sky-icon [icon]="buttonType" size="lg"></sky-icon>
Copy link
Contributor

Choose a reason for hiding this comment

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

Multiple line attributes 😉
This applies to the other files too. I don't want to clutter with one kind of style comment

src/modules/grid/grid.component.html Outdated Show resolved Hide resolved
src/modules/link-records/link-records-item.component.html Outdated Show resolved Hide resolved
src/modules/link-records/link-records-item.component.scss Outdated Show resolved Hide resolved
Copy link
Contributor

@blackbaud-conorwright blackbaud-conorwright 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 to me 😄

@codecov-io
Copy link

codecov-io commented Sep 19, 2018

Codecov Report

Merging #1987 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1987      +/-   ##
==========================================
+ Coverage   99.94%   99.94%   +<.01%     
==========================================
  Files         423      423              
  Lines        9266     9296      +30     
  Branches     1384     1391       +7     
==========================================
+ Hits         9261     9291      +30     
  Misses          5        5
Impacted Files Coverage Δ
src/modules/icon/icon.component.ts 100% <ø> (ø) ⬆️
src/modules/tokens/tokens.module.ts 100% <100%> (ø) ⬆️
src/modules/select-field/select-field.module.ts 100% <100%> (ø) ⬆️
src/modules/fileattachments/file-item.component.ts 100% <100%> (ø) ⬆️
...ector-action/list-column-selector-action.module.ts 100% <100%> (ø) ⬆️
src/modules/tiles/tile/tile.module.ts 100% <100%> (ø) ⬆️
...odules/link-records/link-records-item.component.ts 100% <100%> (ø) ⬆️
src/modules/grid/grid.module.ts 100% <100%> (ø) ⬆️
src/modules/link-records/link-records.module.ts 100% <100%> (ø) ⬆️
src/modules/paging/paging.module.ts 100% <100%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2dac17...5ae123b. Read the comment docs.

@Blackbaud-TrevorBurch
Copy link
Member Author

I removed all old modal screenshots as the fixed font size of the close button was causing a couple to fail consistently on the build machine (they passed locally but I'm guessing it was a resolution thing).

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch merged commit 8a29a0a into master Sep 19, 2018
@Blackbaud-AlexKingman Blackbaud-AlexKingman deleted the 1983-icon-style-issues branch November 12, 2018 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants