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

RC > Removed SKY CSS import #443

Merged
merged 11 commits into from
Aug 9, 2018
Merged

Conversation

Blackbaud-SteveBrush
Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush commented Jul 26, 2018

  • Replaced runtime and karma style loaders with those released in @skyux/theme
  • Removed all references to sky.css.

To test, run against the template's builder-dev-rc branch, here:
https://github.com/blackbaud/skyux-template/tree/builder-dev-rc

@codecov-io
Copy link

codecov-io commented Jul 26, 2018

Codecov Report

Merging #443 into rc-2.0.0 will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           rc-2.0.0     #443     +/-   ##
===========================================
+ Coverage     99.32%   99.42%   +0.1%     
===========================================
  Files            73       72      -1     
  Lines          1912     1899     -13     
  Branches        297      297             
===========================================
- Hits           1899     1888     -11     
+ Misses           13       11      -2
Flag Coverage Δ
#builder 100% <ø> (ø) ⬆️
#runtime 96.07% <ø> (+0.5%) ⬆️
#srcapp 100% <ø> (ø) ⬆️
Impacted Files Coverage Δ
src/app/app.component.ts 100% <ø> (ø) ⬆️
lib/sky-pages-module-generator.js 100% <ø> (ø) ⬆️
runtime/viewport.service.ts 100% <0%> (+50%) ⬆️

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 85c47c2...4827483. Read the comment docs.

// when the font is loaded unless a known character with a different width
// than the default is not specified.
fontAwesome.load('\uf0fc', LOAD_TIMEOUT),
blackbaudSans.load(null, LOAD_TIMEOUT)

Choose a reason for hiding this comment

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

I had quite a few comments typed up before I realized this file is different from https://github.com/blackbaud/skyux-builder/blob/master/runtime/style-loader.ts

For clarification, this file is only called from https://github.com/blackbaud/skyux-builder/blob/19943499ea387f11965c6ccb9c4d35d6cc973f22/utils/spec-styles.js? and spec-styles is used at https://github.com/blackbaud/skyux-builder/blob/master/config/karma/shared.karma.conf.js?

If this is the case, and we're removing the automated loading of the Blackbaud Sans font, I wonder should we instead just remove the blackbaudSans.load() call?

Tangentially related, should we do the same thing to the runtime side?

@Blackbaud-SteveBrush
Copy link
Member Author

@Blackbaud-BobbyEarl This is ready for another look.

package.json Outdated
@@ -39,6 +39,7 @@
"@angular/platform-browser": "^4.3.6",
"@angular/platform-browser-dynamic": "^4.3.6",
"@angular/router": "^4.3.6",
"@skyux/theme": ">3.0.0-alpha.1",

Choose a reason for hiding this comment

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

Should this be >=?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch!

@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 8c20e03 into rc-2.0.0 Aug 9, 2018
Blackbaud-DiHuynh pushed a commit to Blackbaud-DiHuynh/skyux-builder that referenced this pull request Jul 10, 2020
* RC > Removed all references to SKY UX, changed dependency structure (blackbaud#419)

* RC > Updated TSLint rules (blackbaud#422)

* Removed legacy omnibar (blackbaud#420)

* RC > Config params as an object; always decode URL params (blackbaud#421)

* RC > Always provide `envId` regardless of permission scope (blackbaud#427)

* RC > Allow SPAs to bundle stylesheets that live outside of `src/app` (blackbaud#428)

* RC > Adjusted dev dependencies (blackbaud#429)

* RC > Fixed ts-helpers for build (blackbaud#434)

* RC > Update from master (blackbaud#425)

* RC > Removed global RxJS imports (blackbaud#438)

* RC > Replaced error component with iframe (blackbaud#436)

* RC > Removed SKY CSS import (blackbaud#443)

* RC > Instrument different directory for libraries (blackbaud#448)

* RC > Do not ignore Protractor Error 199 (blackbaud#435)

* RC > Merged master (blackbaud#444)

* RC > Merge master (blackbaud#454)

* RC > Upgrade Angular, RxJS, TypeScript (blackbaud#495)

* RC > Moved auth-client to peer dependency; fixed `skyux watch` (blackbaud#503)

* Disabled webpack host check (blackbaud#505)

* Replaced JSHint with ESLint. (blackbaud#506)

* RC > Merge master (blackbaud#508)

* RC > Changed name of NPM package (blackbaud#501)
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.

3 participants