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

Add option for disabling VCS colors #744

Closed
wants to merge 1 commit into from

Conversation

maxpoletaev
Copy link

2016-02-21 01-50-01 settings users maxim projects simply simply-backend

@lee-dohm
Copy link
Contributor

Rather than all this code, that tests would need to be written for, I feel it would be much less risky to implement something like this:

First, add this to the stylesheet in the package:

// remove Git colors from tree-view
.tree-view.vcs-disabled {
  .status-modified,
  .status-added,
  .status-renamed,
  .status-removed {
    color: inherit !important; // remove diff color

    &.directory > .list-item {
      color: inherit !important; // remove diff color
    }
    &.directory.selected > .list-item {
      color: @text-color-selected !important; // add selected color
    }
  }
}

Second, add code that monitors the configuration and either adds or removes the .vcs-disabled class to the top tree view ol element:

screen shot 2016-02-21 at 12 56 40 pm

I tested this manually and it seems to work pretty well.

@maxpoletaev
Copy link
Author

Your solution is more safe but dirty (!important is anti-pattern) and incompatible with some of atom themes. For sample: native-ui theme use pseudo elements for displaying status.

Currently I just remove subscription from git repository to disabling updateStatus calls.
As less risky alternative solution I may suggest don't add status class here if option is disabled. What do you think?

@lee-dohm
Copy link
Contributor

By not adding the status classes, you're mixing function with presentation. I feel that the functionality should remain exactly the same and only the presentation should change. This is what leads me to a CSS-only (or very nearly so) solution.

@maxpoletaev
Copy link
Author

This does't contradict for separating functionality and representation.
See implementation this option in the tabs package. It is the same that I suggest.

@lee-dohm
Copy link
Contributor

I can have some other people from the team take a look once this has automated tests to help us ensure that it doesn't get broken in a future update.

@garygreen
Copy link

@lee-dohm imo your suggestion doesn't really turn off VCS colours, the classes are still being added which unnecessarily impacts performance.

@winstliu
Copy link
Contributor

Closing until tests are added.

@winstliu winstliu closed this Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants