Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

refactor: Revert CSS privatization. #8874

Closed

Conversation

topherfangio
Copy link
Contributor

@topherfangio topherfangio commented Jun 27, 2016

Revert previous changes to privatize much of our CSS.

See public announcement Revisiting the decision to rename private CSS classes in 1.1

Also fixes an erroneously passing chips test.

@topherfangio topherfangio added the needs: review This PR is waiting on review from the team label Jun 27, 2016
@clshortfuse
Copy link
Contributor

clshortfuse commented Jun 27, 2016

I ran a grep and found a few you've missed

src/components/datepicker/datePicker.scss:45:._md-datepicker-floating-label {
src/components/datepicker/js/datepickerDirective.js:160:  var INPUT_CONTAINER_CLASS = '_md-datepicker-floating-label';
src/components/progressLinear/progress-linear.js:65:  var DISABLED_CLASS = "_md-progress-linear-disabled";
src/components/progressLinear/progress-linear.spec.js:112:      expect(progress.hasClass('_md-progress-linear-disabled')).toBe(true);
src/components/progressLinear/progress-linear.scss:12:  &._md-progress-linear-disabled {
src/components/progressLinear/progress-linear.scss:91:    ._md-progress-linear-disabled & {
src/components/slider/slider.js:197:    var container = angular.element($mdUtil.getClosest(element, '_md-slider-container', true));
src/components/progressCircular/progress-circular.spec.js:106:    expect(element.hasClass('_md-progress-circular-disabled')).toBe(true);
src/components/progressCircular/progress-circular.scss:12:    &._md-progress-circular-disabled {
src/components/progressCircular/js/progressCircularDirective.js:61:  var DISABLED_CLASS = '_md-progress-circular-disabled';
src/components/toolbar/toolbar.js:76:        element.addClass('_md-toolbar-transitions');     // adding toolbar transitions after digest
src/components/toolbar/toolbar.scss:33:  &._md-toolbar-transitions {
src/components/panel/panel.spec.js:8:  var HIDDEN_CLASS = '_md-panel-hidden';
src/components/panel/panel.spec.js:9:  var FOCUS_TRAPS_CLASS = '._md-panel-focus-trap';
src/components/panel/panel.spec.js:10:  var FULLSCREEN_CLASS = '_md-panel-fullscreen';
src/components/panel/panel.spec.js:11:  var BACKDROP_CLASS = '._md-panel-backdrop';
src/components/panel/panel-theme.scss:5:  &._md-panel-backdrop.md-THEME_NAME-theme {
src/components/panel/panel.js:130: *     Applies the class `._md-panel-fullscreen` to the panel on open. Defaults
src/components/panel/panel.js:598:var MD_PANEL_HIDDEN = '_md-panel-hidden';
src/components/panel/panel.js:601:    '<div class="_md-panel-focus-trap" tabindex="0"></div>');
src/components/panel/panel.js:1168:      self._panelEl.addClass('_md-panel-fullscreen');
src/components/panel/panel.js:1249:            open: '_md-opaque-enter',
src/components/panel/panel.js:1250:            close: '_md-opaque-leave'
src/components/panel/panel.js:1256:        panelClass: '_md-panel-backdrop',
src/components/panel/panel.js:1438:    this.addClass('_md-panel-shown');
src/components/panel/panel.js:1466:    this.removeClass('_md-panel-shown');
src/components/panel/panel.js:2156:        transitionInClass: '_md-panel-animate-enter'
src/components/panel/panel.js:2166:        transitionInClass: '_md-panel-animate-enter'
src/components/panel/panel.js:2176:        transitionInClass: '_md-panel-animate-enter'
src/components/panel/panel.js:2221:        transitionInClass: '_md-panel-animate-leave'
src/components/panel/panel.js:2231:        transitionInClass: '_md-panel-animate-scale-out _md-panel-animate-leave'
src/components/panel/panel.js:2241:        transitionInClass: '_md-panel-animate-fade-out _md-panel-animate-leave'
src/components/panel/panel.scss:9:._md-panel-hidden {
src/components/panel/panel.scss:13:._md-panel-fullscreen {
src/components/panel/panel.scss:23:._md-panel-shown .md-panel {
src/components/panel/panel.scss:32:  &._md-panel-shown {
src/components/panel/panel.scss:39:  &._md-panel-animate-enter {
src/components/panel/panel.scss:44:  &._md-panel-animate-leave {
src/components/panel/panel.scss:49:  &._md-panel-animate-scale-out,
src/components/panel/panel.scss:50:  &._md-panel-animate-fade-out {
src/components/panel/panel.scss:54:  &._md-panel-backdrop {
src/components/panel/panel.scss:60:  &._md-opaque-enter {
src/components/panel/panel.scss:65:  &._md-opaque-leave {
src/components/navBar/navBar.js:109:          '<ul class="_md-nav-bar-list" layout="row" ng-transclude role="listbox"' +
src/components/navBar/navBar.js:170:    return self._navBarEl.querySelectorAll('._md-nav-button').length;
src/components/navBar/navBar.js:237:  this._inkbar.toggleClass('_md-left', newIndex < oldIndex)
src/components/navBar/navBar.js:238:      .toggleClass('_md-right', newIndex > oldIndex);
src/components/navBar/navBar.js:392:        '<md-button ng-if="ctrl.mdNavSref" class="_md-nav-button md-accent"' +
src/components/navBar/navBar.js:396:          '<span ng-transclude class="_md-nav-button-text"></span>' +
src/components/navBar/navBar.js:398:        '<md-button ng-if="ctrl.mdNavHref" class="_md-nav-button md-accent"' +
src/components/navBar/navBar.js:402:          '<span ng-transclude class="_md-nav-button-text"></span>' +
src/components/navBar/navBar.js:404:        '<md-button ng-if="ctrl.mdNavClick" class="_md-nav-button md-accent"' +
src/components/navBar/navBar.js:408:          '<span ng-transclude class="_md-nav-button-text"></span>' +
src/components/navBar/navBar.js:426:          mdNavItem.name = angular.element(element[0].querySelector('._md-nav-button-text'))
src/components/navBar/navBar.js:430:        var navButton = angular.element(element[0].querySelector('._md-nav-button'));
src/components/navBar/navBar.js:505:  return this._$element[0].querySelector('._md-nav-button');
src/components/navBar/navBar.scss:11:._md-nav-bar-list {
src/components/navBar/navBar.scss:23:.md-button._md-nav-button {
src/components/navBar/navBar.scss:48:  &._md-left {
src/components/navBar/navBar.scss:52:  &._md-right {
src/components/navBar/navBar-theme.scss:8:  .md-button._md-nav-button.md-unselected {
src/components/navBar/navBar.spec.js:29:    tabContainer = angular.element(el[0].querySelector('._md-nav-bar-list'));
src/components/chips/chips-theme.scss:35:    &._md-chip-editing {
src/components/chips/chips.spec.js:220:          var removeContainer = wrap[0].querySelector('._md-chip-remove-container');
src/components/chips/chips.spec.js:273:          var containers = wrap[0].querySelectorAll("._md-chip-input-container");
src/components/chips/chips.spec.js:276:          var removeContainer = wrap[0].querySelector('._md-chip-remove-container');
src/components/chips/chips.spec.js:292:          var containers = wrap[0].querySelectorAll("._md-chip-remove-container");
src/components/chips/chips.spec.js:300:          containers = wrap[0].querySelector("._md-chip-remove-container");
src/components/chips/chips.spec.js:370:          var removeContainer = wrap[0].querySelector('._md-chip-remove-container');
src/components/chips/chips.spec.js:375:          removeContainer = wrap[0].querySelector('._md-chip-remove-container');
src/components/chips/js/chipsController.js:182:  return !!this.$element[0].getElementsByClassName('_md-chip-editing').length;
src/components/chips/js/chipController.js:69:    this.getChipContent().addClass('_md-chip-content-edit-is-enabled');
src/components/chips/js/chipController.js:106:  this.$element.removeClass('_md-chip-editing');
src/components/chips/js/chipController.js:153:  this.$element.addClass('_md-chip-editing');
src/components/chips/chips.scss:107:    &._md-chip-content-edit-is-enabled {
src/components/fabSpeedDial/fabController.js:145:        $animate.addClass($element, '_md-animations-ready').then(function() {
src/components/fabSpeedDial/fabSpeedDial.scss:28:  ._md-css-variables {
src/components/fabSpeedDial/fabSpeedDial.js:116:      element.prepend('<div class="_md-css-variables"></div>');
src/components/fabSpeedDial/fabSpeedDial.js:125:      if (element.hasClass('md-animations-waiting') && !element.hasClass('_md-animations-ready')) {
src/components/fabSpeedDial/fabSpeedDial.js:137:      var variablesElement = el.querySelector('._md-css-variables');
src/components/fabSpeedDial/fabSpeedDial.js:222:      var variablesElement = el.querySelector('._md-css-variables');
src/components/list/list.js:334:          tEl.addClass('_md-button-wrap');
src/components/list/list.js:431:            isButtonWrap  = $element.hasClass('_md-button-wrap'),
src/components/list/list.spec.js:153:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:179:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:205:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:230:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:255:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:272:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:292:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:317:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.spec.js:349:    expect(listItem).toHaveClass('_md-button-wrap');
src/components/list/list.scss:136:  &._md-button-wrap {
src/components/menu/js/menuDirective.js:205:        menuEl.classList.add('_md-nested-menu');
src/components/toast/toast.scss:181:    &._md-start {
src/components/toast/toast.scss:185:    &._md-end {

I just wanted to play with grep :-P I'm going to try my hand at a regex for this

Actually grep -rn "_md-" src/ seems to work fine for this. I updated this comment with the results

@topherfangio
Copy link
Contributor Author

@clshortfuse Thanks for the printout. Most of those are actually legitimate though. I only reverted the ones that we changed after the 1.0 release. Some (especially the panel/navbar) were private classes that will be added in 1.1.0, but weren't available in 1.0.0, so I left them as-is.

Additionally, there were two classes that I think definitely should be private as they are for use by the internal workings of the FAB speed dial, so I left them.

That said, it looks like I did miss a few with the chips and the chip remove container, so I'm working to fix those. Incidentally, that's why the build is failing, but I need to touch base with @devversion about one of the changes real fast before I can fix it.

@ThomasBurleson I'm marking this as "Needs Work", but I should have it fixed/ready early tomorrow.

@topherfangio topherfangio added needs: work and removed needs: review This PR is waiting on review from the team labels Jun 28, 2016
@topherfangio topherfangio added needs: review This PR is waiting on review from the team and removed needs: work labels Jun 28, 2016
@topherfangio
Copy link
Contributor Author

@ThomasBurleson I fixed the failing tests; this should now be ready for review.

@ThomasBurleson
Copy link
Contributor

@topherfangio - LGTM. Merging on July 11.

See public announcement Revisiting the decision to rename private CSS classes in 1.1

Revert previous changes to privatize much of our CSS.

> See public announcement [**Revisiting the decision to rename private CSS classes in 1.1**](https://groups.google.com/forum/#!msg/ngmaterial/mLJrRW9qrLA/C_Ni3LSrBQAJ)

Also fixes an erroneously passing chips test.
@david-gang
Copy link
Contributor

Hi @topherfangio ,

Do you have a list of all css classes which were reverted from private to public?
We already changed all classes, and now this would mean that we have to review the whole application again.
Getting a list would enable us to create a script which makes the automatic conversion.

Thanks,
David

@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Jul 12, 2016

@david-gang - I do not understand "we already changed all classes". Which classes?

Do you mean you were overriding private CSS and when those were privatized you added _ prefixes to your custom overrides?

If yes, then you have obviated the entire point of private CSS... as those are classes and styles you should NOT be extending or overriding.

@david-gang
Copy link
Contributor

Yes @ThomasBurleson but at the time I used them they were not private, so I relied on them and now that they are public again I can use them again, so I just need a list of all css variables where the privatization was reverted

@ThomasBurleson
Copy link
Contributor

@david-gang - regardless of our removal of the "_" prefix, I would recommend that you should not use these classes unless you absolutely need to.

We are working to prepare a list of changed CSS classnames and types affected.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: review This PR is waiting on review from the team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants