-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(demos): Add theme switcher to theme demo page #1975
Conversation
commit 1e8970f Author: Matty Goo <[email protected]> Date: Wed Jan 17 09:19:54 2018 -0800 chore(text-field): moved textarea sass into private mixins (#1942) commit bffadd3 Author: Andrew C. Dvorak <[email protected]> Date: Wed Jan 17 09:09:56 2018 -0800 style(demos): Clean up theme demo Sass and HTML (#1973) - Remove unused code - IE 11 compatible property values (`unset` -> `auto`) - Fix duplicate IDs - Use `--stroked` buttons instead of `--raised` for checkbox demo - Rename CSS classes to be more BEM-y - Rename and reorganize Sass demo vars - Reword text-field labels and helper text for clarity - Remove `getAll()` - Inline some JS vars commit 3a1786f Author: Dominic Carretto <[email protected]> Date: Wed Jan 17 11:37:03 2018 -0500 fix(slider): Add MDCSliderFoundation export (#1959) commit 815eade Author: Simon Olofsson <[email protected]> Date: Wed Jan 17 17:36:07 2018 +0100 docs(menu): Remove obsolete `mdc-simple-menu--open-from` classes. (#1927) commit 6078784 Author: Chafic Najjar <[email protected]> Date: Wed Jan 17 18:26:00 2018 +0200 docs: Rewrite all instances of "MDC-Web" as "MDC Web" (#1960) Resolves #1924 commit e87c110 Author: pndewit <[email protected]> Date: Wed Jan 17 17:23:03 2018 +0100 docs(drawer): Fix missing link to temporary drawer demo (#1922) commit 003dff4 Author: Chafic Najjar <[email protected]> Date: Wed Jan 17 18:20:35 2018 +0200 docs: Fix broken links to AngularJS's Git commit guidelines (#1888)
Codecov Report
@@ Coverage Diff @@
## master #1975 +/- ##
=======================================
Coverage 99.43% 99.43%
=======================================
Files 84 84
Lines 3717 3717
Branches 485 485
=======================================
Hits 3696 3696
Misses 21 21 Continue to review full report at Codecov.
|
…demo-v2 # Conflicts: # demos/theme/_theme-shared.scss # demos/theme/index.html
Will make unit testing easier if we decide we need it for demo JS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
demos/dom.js
Outdated
* @return {!Array<!Element>} | ||
*/ | ||
export function getAll(query, opt_root) { | ||
opt_root = opt_root || document; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop opt_
prefix and inline as default parameter? getAll(query, root = document)
The opt_
prefix is discouraged now in favor of default parameters. (See reference)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
demos/interactivity.js
Outdated
* @return {?Element} | ||
* @protected | ||
*/ | ||
getElementById_(id, opt_root) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this more complicated than simply calling document.getElementById
?
For that matter, are these various root
parameters ever passed anything other than document
in practice, or do we have some cases of YAGNI in here?
Also, same idea here and below RE opt_root
-> root = ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
demos/theme/index.js
Outdated
this.flashCurrentSection_(); | ||
} | ||
|
||
// TODO(acdvorak): Replace page URL hash with current section on scroll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder that this TODO is here, whether or not you want to deal with it right now; given that we have links in each heading I don't think it's critical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed TODO. Thanks!
@@ -61,10 +66,35 @@ body { | |||
background-color: $mdc-theme-background; | |||
} | |||
|
|||
figure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be redundant of styles 10 lines above this. Remove this or remove those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
- Remove `opt_` prefix from function arg names - Use default values (except in uri.js and theme-loader.js, which are not transpiled) - Remove unneeded `|undefined` from optional param JSDoc - Rename var for clarity
This PR also adds hot reloading of stylesheets that have a
data-hot
attribute.Currently, only the theme demo page has a
<link rel="stylesheet" href="..." data-hot>
element, but I plan to create follow-up PRs to add hot swapping to the remaining demo pages. Hot swapping was broken when we switched to CSS files in #1916.When Webpack starts recompiling our Sass, the theme demo page displays a linear-progress bar in the toolbar until compilation finishes, at which point it automatically swaps in the new stylesheet.
I tested this PR in: