Merge Bootstrap + Core styles

[simurai]

⚠️ This is in parallel to #11839 + #11858. Don't merge all PRs, only pick one. ⚠️

This PR does:

1. Remove bootstrap dependency
2. Merge Bootstrap 3.3.6 + some Core styles and publish it as atom-ui module
3. Add atom-ui dependency

Refs: #6351, #8237

Stays in core atom/atom/static:

• Octicons
• Normalize
• Scaffolding
• Cursors
• Panels
• Panes
• Syntax
• text-editor-light
• workspace-view

Moved + merged into the atom/atom-ui package:

• Tooltips
• Buttons + Button Groups
• Lists
• Close
• Code
• Nav
• Scaffolding
• Utilities
• Alerts
• Tables
• Forms
• Type
• Normalize
• Clean up some un-used styles. Like IE hacks etc.

Removed:

• 🔥 grid
• 🔥 input-groups
• 🔥 labels
• 🔥 thumbnails

Motivation

Currently we load Bootstrap and then try to override it in core. Merging them allows more control and adjustments can be made directly. Also, moving out of core and into a separate package feels less intimidating and can easier be rolled back in package.json.

Concerns

Some styles got removed, so there is a chance that something might look unstyled or off. Packages bundled with Atom should be fine, but it's hard to predict if community packages rely on any of the removed styles. Maybe in the Beta phase we can catch some regressions.

Affected packages:

• autocomplete-plus fixed by reverting some refactoring.
• godoc uses .close. Fixed by adding .close too.

[simurai]

@thomasjo Did I get this npm thing right?

I had to import the atom-ui files in atom.less (instead of bootstrap.less), since atom-ui uses some of the ui-variables. Maybe ok.

[simurai]

One downside I found is that the "live-reload" doesn't work and you have to manually reload when making changes to the atom-ui package. Maybe ok and still easier than building Atom.

[simurai]

Although the build fails (I think unrelated), this branch would be ready for a test drive. Things to test:

1. Checkout this branch
2. Run script/build and see if there are any errors when building Atom
3. See if the UI looks still the same and nothing is unstyled, broken or off in some ways.

/cc @atom/feedback

[simurai]

Ouch.. apparently adding some padding to highlights causes this find-and-replace spec to fail. 🙈

[thomasjo]

@simurai Once bootstrap.less is removed, this looks good to me! ✨ I'll keep testing for a day or ✌️

[thomasjo]

Been using this for a few days now, and haven't noticed any regressions. Let's 🔥 bootstrap.less and 🚢 this?

/cc @atom/feedback

[simurai]

Let's 🔥 bootstrap.less

I guess it's this:

atom/src/theme-manager.coffee

Line 173 in f48069c

@requireStylesheet('../static/bootstrap')

?

Also, searched for bootstrap and found these places that probably should be cleaned up:

• atom/build/tasks/prebuild-less-task.coffee

  Lines 11 to 36 in f48069c

  	compileBootstrap = ->
  	appDir = grunt.config.get('atom.appDir')
  	bootstrapLessPath = path.join(appDir, 'static', 'bootstrap.less')
  	bootstrapCssPath = path.join(appDir, 'static', 'bootstrap.css')
  	lessCache = new LessCache
  	cacheDir: temp.mkdirSync('atom-less-cache')
  	fallbackDir: grunt.config.get('prebuild-less.options.cachePath')
  	syncCaches: true
  	resourcePath: path.resolve('.')
  	bootstrapCss = lessCache.readFileSync(bootstrapLessPath)
  	grunt.file.write(bootstrapCssPath, bootstrapCss)
  	rm(bootstrapLessPath)
  	rm(path.join(appDir, 'node_modules', 'bootstrap', 'less'))
  	cacheMisses += lessCache.stats.misses
  	cacheHits += lessCache.stats.hits
  	importFallbackVariables = (lessFilePath) ->
  	if lessFilePath.indexOf('static') is 0
  	false
  	else
  	true
  	grunt.registerMultiTask 'prebuild-less', 'Prebuild cached of compiled Less files', ->
  	compileBootstrap()

• atom/build/tasks/build-task.coffee

  Lines 62 to 67 in ca04138

  	path.join('bootstrap', 'docs')
  	path.join('bootstrap', 'dist')
  	path.join('bootstrap', 'fonts')
  	path.join('bootstrap', '_config.yml')
  	path.join('bootstrap', '_includes')
  	path.join('bootstrap', '_layouts')

Not sure about:

• atom/src/task.coffee

  Lines 71 to 82 in f48069c

  	taskBootstrapRequire = "require('#{require.resolve('./task-bootstrap')}');"
  	bootstrap = """
  	#{compileCacheRequire}.setCacheDirectory('#{compileCachePath}');
  	#{taskBootstrapRequire}
  	"""
  	bootstrap = bootstrap.replace(/\\/g, "\\\\")
  	taskPath = require.resolve(taskPath)
  	taskPath = taskPath.replace(/\\/g, "\\\\")
  	env = _.extend({}, process.env, {taskPath, userAgent: navigator.userAgent})
  	@childProcess = ChildProcess.fork '--eval', [bootstrap], {env, silent: true}

Should we do that in a separate PR? Currently bootstrap.less is just empty.

[thomasjo]

Should we do that in a separate PR? Currently bootstrap.less is just empty.

Sure, we could remove those references in a separate PR to limit the scope of this one. But simply commenting out the lines in bootstrap.less and not removing it feels a bit "messy". I'll see how much work is involved in simply killing it with 🔥

[thomasjo]

@simurai I pushed a commit that nukes bootstrap.less and (hopefully) all references to it.

@atom/feedback Some 👀 on this would be great. I believe this PR is ready to 🚢

[thomasjo]

🚀