Move breakpoint settings to theme and module *.breakpoints.yml files at root level

The UI for sizes is part of the mapping and already implemented in the patch.

Various people mentioned that they'd rather like to have the breakpoint definitions in the theme's .info.yml file.

This would inherently cause them to be "theme info/settings" instead of configuration, and thus, they'd still be editable after a theme has been installed.

Based on that consideration, I think a breakpoint UI should be limited to be a simple "YAML code generator"; i.e.,

1. it automatically loads the currently defined breakpoints from the theme's .info.yml file
2. you can edit/tweak them in the UI…
3. you click an "Export" button that simply shows a textarea from which you can copy the new definitions into your .info.yml file.

It's my impression that this is what >90% of all themers expect. — Especially the aspect of being able to change the breakpoint definitions after installation, as well as the aspect of ensuring total ownership for themes, because the definitions actually have to be resembled in the theme's CSS, too.

I am still not sure about this, I think I mentioned it in every Breakpoint UI discussion thus far. I don't really see why we have another one, but as always my argumentation:

1) The UI that we expose for Breakpoints, or earlier tries so far - have been incredibly difficult and complex for users to understand. This proposal seems to show no process or understanding, on how to resolve that.
2) The step for themers to do this in the theme its info.yml file is very small. It seems counter intuitive to me, for such a "programming" heavy task to be moved towards the UI. Tweaking happens in concurrence with CSS changes. Why would we force them to go to the UI for breakpoints?

At this stage in the release cycle (near the end), of polishing we should not be introducing major new UI's. Especially knowing it will inherently have many UX difficulties. It should be totally possible to introduce this in 8.1, if we take care of the breakpoints being in the info file step.

I wrote down some thoughts and concerns before in #2260061-7: Responsive image module does not support sizes/picture polyfill 2.2 and #2260061-13: Responsive image module does not support sizes/picture polyfill 2.2, but what it comes down to is this: I think we should not have a Breakpoints UI.

In the new picture element, the breakpoints listed in the sizes attribute should mirror your CSS’ breakpoints exactly (says Eric Portis). To achieve that, Drupal (read: the PHP involved in rendering picture elements) must either have knowledge about the theme's breakpoints, or must control the theme's breakpoints. So the question is: who is leading, and who is following? Should Drupal's breakpoint config follow the theme's CSS (Sass / Less / Stylus / whatever-comes-next) code, or should the CSS code follow the configuration set in the Breakpoints UI?

From a UX point of view, if Drupal offers a breakpoint UI users would expect it to control both the theme and responsive images (but that kills DX, see below). If the config from a breakpoint UI is used for responsive images only (like the D7 Breakpoints module), we'll have to tell users: "This is where you set breakpoints, but not really because it will not change your layout, and you're not allowed to touch this anyway because the settings must match the media queries of your theme." In other words, it would be a UI that does not produce the expected result, and makes it way too easy to get things wrong.

From a DX point of view, I strongly feel that themes should be leading, and themes should make their breakpoints configuration available to other modules that need the info. I have worked with a D7 base theme that tries to make UI-configured media queries available in the Sass code, and IMHO it was a nightmare. Front end devs should be free to use the front end tools of their choice without having to worry about integrating breakpoint config coming from Drupal. Themers who are skilled enough to write CSS or Sass, must surely be able to write down the media queries in the .info.yml file, thus exposing the info to Drupal modules.

mdrummond: Completely agree. Move code + clear cache sounds like a good deployment workflow.

I do realize that matching breakpoint config to theme media queries is still the responsibility of a human being, capable of making mistakes. I'm not expecting magic of any kind. Our job is just to make it easy to get everything matched up (while a UI would only make it easy to get it wrong).

Slightly off-topic, but worth a mention: I just realized that the mapping of breakpoints to image styles will have to be configured per theme, just like the placement of blocks in regions is configured separately for each theme.

I agree we don't need a UI for breakpoints, but not 100% sure about picture mappings per theme, especially since we now have support for sizes and you can do things like the following, which does not use any breakpoint at all. Besides that adding theme the a picture mapping will complicate the UI of picture mappings even more.

<picture>
    <source sizes="100vw" srcset="pic400.jpg 400w, pic800.jpg 800w, pic1600.jpg 1600w">
    <img src="pic400.jpg" alt="The president giving an award.">
  </picture>

Screenshot so you know what I'm talking about (keep in mind that the theme has to define an empty breakpoint)

The media query in the screenshot is not used in the theme, only for this mapping

Right, we can display their machine name or the theme they or defined in.

Keep in mind that modules can define breakpoints as well, the new nav module uses this, so we need a way to handle this as well, but now idea if this is possible with the proposal of @sun.

(Just so that we can find this issue from the CMI side.) :)

Since it has been brought up here the relationship between both breakpoint definitions as well as responsive image mapping definitions and their source needs to be cleared up. A mapping can be created by a module, a theme, and through the UI. A breakpoint can only be created by a module or a theme. Though you can change a mapping's breakpoint selection, you can not delete a mapping once created. A breakpoint definition is deleted when its source is disabled. Since a mapping can point to any breakpoint from any source, what do you do if the currently active mapping has its breakpoint definitions deleted? Or if the breakpoint definition were changed by an update to the module or theme that created it? It just seems there is something basically wrong here abut how responsive images consume breakpoints.

#16:

1/ we need to allow a user to delete a mapping
2/ if you delete a breakpoint definition that is still used by a mapping, thing s will probably break, but I guess that's similar for other components as well. If you disable a link field while it is used in a content type and/or in a view it will break as well.

#18 Sorry about the override.

I created an issue for this, #2203459: responsive_image mapping doesn't implement 'delete' function, but it is more than allowing a delete operation in the UI. It needs to be discussed(over there) as to what to do if a module or theme is disabled.

1. I've the impression we've reached agreement on how things are supposed to work.

   At the same time,

2. This discussion apparently revealed a huge can of worms encompassing a dozen of architectural flaws in the current system.

   Therefore, I wonder whether

3. This issue should be promoted to a beta blocker.

To clarify that can of worms:

1. Some other stuff ([responsive] image styles, picture, etc) is apparently tied to breakpoints (currently config), but breakpoints should be definitions. Dependency (type) error.

2. Breakpoints must be defined per-theme, not globally, because it's impossible to "force" breakpoints onto a(ny) theme.

3. D8 seemingly even introduced a highly questionable facility of modules being able to define breakpoints, completely detached from any knowledge of any theme.

   Such a 3D model - in which two dimensions are hard-coded in independent CSS files - is impossible to work out in practice.

4. ...

The sum of these aspects is not just one or two independent bugs — it's a series of functionalities that has been tacked onto an architecture that is missing a fundamental key constraint/filter: Nothing is global, everything MUST be restricted to a particular theme.

This is not a critical issue (nor should it be) and therefore does not block a beta release. Also, a reminder that the beta blocker tag should only be added by a core maintainer.

Switching to "beta deadline" which is the correct tag here.

And beta target for visibility, because the breakpoint configuration implementation is something that would we should fix if we have time.

Yes, I meant pretty much everything the way you described it.

I'd just like to use clear terminology: Breakpoints should be defined by a theme. The theme should be the owner, controller, and originating source. Breakpoints should not be configuration that is "dependent" on a theme; breakpoints should be definitions of a theme, because breakpoints are hard-coded in the asset files of the theme.

This implicitly means that breakpoints are no longer configured globally, independent from a theme. Instead, they are explicitly defined by a theme. If a theme defines no breakpoints, then there are no breakpoints.

Since breakpoints are no longer configuration, but definitions in code, their maintenance will need to change. The most sensible approach I can think of is to (1) collect all definitions, (2) store all current definitions in a state record, and (3) if a later rebuild identifies that definitions were added/changed/removed, then corresponding hooks/events are invoked, so as to allow other modules to react accordingly (e.g., responsive_image).

This patch moves from config entities to \Drupal::states()

Works, but could probably use some tests:

• Info files are checked when the breakpoint module is enabled
• Info files of modules that get enabled afterwards are checked when they get enabled
• Info files are re-checked on cache clear

Above patch does not yet implement #24-3 (hooks/events when breakpoints are added/changed/deleted)

I'll start working on implementing the insert/update/delete hooks

Did some manual testing as well to make sure everything worked after rerolling this patch because of the PSR-4 commit.
Hooks are in place, breakpoint.api.php added to document the hooks.

1. +++ b/core/modules/breakpoint/breakpoint.module
   @@ -62,11 +71,185 @@ function breakpoint_group_select_options() {
   +/**
   + * Implements hook_themes_enabled()
   + */
   

   typo, is hook_themes_disabled

2. +++ b/core/modules/breakpoint/tests/themes/breakpoint_test_theme/breakpoint_test_theme.info.yml
   @@ -4,3 +4,52 @@ description: 'Test theme for breakpoint.'
   +  theme.breakpoint_test_theme.mobile:
   +    id: theme.breakpoint_test_theme.mobile
   +    name: mobile
   +    label: mobile
   +    mediaQuery: '(min-width: 0px)'
   +    source: breakpoint_test_theme
   +    sourceType: theme
   +    weight: 0
   +    multipliers:
   +      1x: 1x
   +    status: true
   +    langcode: en
   

   have a look if we can simplify this, I guess we only need name, label, mediaQuery, weight and multipliers. Since breakpoints can not change groups anymore (they are tied to a theme or module), it should be possible to load all of them at once.

3. +++ b/core/modules/responsive_image/responsive_image.module
   @@ -200,7 +199,7 @@ function theme_responsive_image($variables) {
   +    $breakpoint = breakpoint_load_breakpoint($breakpoint_name);
   

   if we simplify the config, we need to load all breakpoints of a group.

new patch with comments of #32 addressed.

Since this is a beta deadline, I think that precludes the need to revisit before release. :) Adding a reference to the "revisit before beta" issue to remove breakpoint from core.

I can give this a review over the next few days.

@Cottser thanks

This needs a reroll, I should have checked yesterday.

Reviewing the latest patch initially and reading through the issue everything makes sense so far, and this is removing a lot of code and still seems to keep quite a good amount of test coverage.

Some minor points below.

1. +++ b/core/modules/breakpoint/breakpoint.module
   @@ -46,10 +43,22 @@ function breakpoint_help($route_name, Request $request) {
   +    $group_id = $module->getType() . '.' . $module->getName() . '.breakpointgroup';
   ...
   +    $group_id = $theme->getType() . '.' . $theme->getName() . '.breakpointgroup';
   
   @@ -62,11 +71,162 @@ function breakpoint_group_select_options() {
   +    $breakpoint_group = \Drupal::state()->get($module->getType() . '.' . $module->getName() . '.breakpointgroup');
   ...
   +    $breakpoint_group = \Drupal::state()->get($theme->getType() . '.' . $theme->getName() . '.breakpointgroup');
   ...
   +      'id' => $extension->getType() . '.' . $extension->getName() . '.breakpointgroup',
   ...
   +  $breakpoint_group = breakpoint_load_breakpoint_group($extension->getType() . '.' . $extension->getName() . '.breakpointgroup');
   

   I wonder whether we should be considering some kind of helper function for these, similar to what Drupal\breakpoint\Entity\BreakpointGroup::id() was doing.

2. +++ b/core/modules/breakpoint/breakpoint.module
   @@ -62,11 +71,162 @@ function breakpoint_group_select_options() {
   + * Save breakpoints from an extension (module or theme).
   ...
   + * Delete breakpoints from an extension (module or theme).
   ...
   + * Load a breakpoint group.
   ...
   + * Read breakpoint groups from all extensions.
   ...
   + * Remove all breakpoint groups.
   

   Minor: I think the first word in the summary lines of these should be plural per https://www.drupal.org/node/1354#functions

Thank you for the review!

Here's a reroll with the points from #39 addressed.

Discussed with @alexpott. In his assessment the current implementation is not releasable, but the upgrade path is doable after beta since we would simply no longer be using CMI for this. So it's just an API change for things that implement breakpoints plus a little cleanup deleting the config entities.

@xjm Do you know if Alex's concerns are with this patch or with the implementation of picture?

You can read more about Alex's thoughts on the breakpoint module in #2120875: Remove breakpoint and picture module from core. I can't really speak for Alex, but last week he said he would be fine with this patch as an alternative to removing breakpoint from core.

@mtift thanks for the feedback, this is a much cleaner solutions, so I'll hope we can quickly move forward

Reroll of #40

Maybe an actual patch would help... (Oops)

reroll
for
#697760: Replace getInfo() in tests with native phpDoc + annotations (following PHPUnit)
which changed tests that this patch was removing.

just removed the tests.

oops. missed a conflict.
took out the use Drupal\breakpoint\Entity\Breakpoint as is done elsewhere in this patch.

rerolled patch: menu_links removed as a dependency from toolbar

rerolled patch...again: menu_links removed as a dependency from toolbar

I'll see if I can get the tests fixed and address all other points mentioned in #67.

New patch, failing test from previous patch was green on my local machine.

Here's the interdiff ;-)

No test coverage or usages of breakpoint_select_options() - needs a followup either to test or remove.

I don't think the hooks make sense. Especially given that breakpoint_rebuild() will fire delete and insert on all the existing breakpoints. We're still treating breakpoints and breakpoint groups as being configurable. I don't think the core implementation should at all. We're just caching the information from the .info.yml files. In fact I don't think we should have all this save/delete logic at all.

Why are we using state and not cache?

Here is a major rewrite of the patch and the breakpoint and responsive_image.

Breakpoints are stored in EXTENSION.breakpoints.yml in the root folder of the theme or module. Breakpoints are discovered and managed by the BreakpointManager which leverages the plugin system in the same way that, for example, contextual links does. Comparing HEAD, the patch #69 there is a considerable saving in complexity. The responsive image mapping has also been rewritten to sensibly stored mappings in a config safe array avoiding any possibility of dots in configuration keys. Methods have been added to the configuration entity to considerably reduced the complexity in using them.

Still @todo ensure that responsive mappings depend on the module or theme that provides the breakpoint group. This will ensure that stale configuration does not remain after uninstalling an extension and also that the user informed when uninstalling.

A contrib module can provide configurable breakpoints by implement an alter hook to change the definitions on the breakpoint plugin manager :)

The interdiff will be super confusing because this is a completely new approach so not providing.

Installed patch on a new clone of D8. Applied ok. After enabling responsive images tried to create a mapping with the bartik breakpoints. Return from 'save' operation said mapping was saved. Log messages said mapping was saved. Mapping exists in config table in database. But admin/configuration/responsive images page still says no mappings exist. Tried to add a field with a responsive image, but the drop down says no mappings. Can't test further at this point.

There are weird things going on after install(see #2307939: Update notifications are not enabled.. But I cleared cache 6 or 7 times with no change. The mapping file I created exists, but the fact it can't be seen by some forms may only be that the existing code is looking for a different naming syntax. And I have tried rebuilding multiple times.

In act, this might be a case of using 'mapping' instead of 'mappings'.

New patch:

• Adds the dependency to the responsive image mappings
• Moves the breakpoints from stark.info.yml to stark.breakpoints.yml
• Clears cached breakpoints when modules or themes are enabled/disabled

@dcrocks I did not ran into any issues following your steps described in #76. The mapping showed up in the field formatter as expected. Maybe you should try a fresh D8 install?

New patch, there was an issue where the cached breakpoint groups didn't get cleared.

+++ b/core/modules/breakpoint/breakpoint.module
@@ -40,33 +37,29 @@ function breakpoint_help($route_name, RouteMatchInterface $route_match) {
+function breakpoint_modules_installed($installed_modules) {
...
+function breakpoint_modules_uninstalled($uninstalled_modules) {
...
+function breakpoint_themes_enabled($theme_list) {
...
+function breakpoint_themes_disabled($theme_list) {

+++ b/core/modules/breakpoint/breakpoint.services.yml
@@ -0,0 +1,6 @@
+      - { name: plugin_manager_cache_clear }

+++ b/core/modules/breakpoint/src/BreakpointManager.php
@@ -0,0 +1,194 @@
+  /**
+   * @var array
+   */
+  protected $breakpointsByGroup;
...
+    $this->setCacheBackend($cache_backend, 'breakpoints:' . $language_manager->getCurrentLanguage()->getId(), array('breakpoints' => TRUE));
...
+  public function getBreakpointsByGroup($group_name) {
...
+      if ($cache = $this->cacheBackend->get($this->cacheKey . ':' . $group_name)) {
+        $this->breakpointsByGroup[$group_name] = $cache->data;
...
+        $this->cacheBackend->set($this->cacheKey . ':' . $group_name, $breakpoints);
+        $this->breakpointsByGroup[$group_name] = $breakpoints;
...
+  public function getGroups() {
+    // Use a double colon so as to not clash with the cache for each group.
+    if ($cache = $this->cacheBackend->get($this->cacheKey . '::groups')) {
...
+    $this->cacheBackend->set($this->cacheKey . '::groups', $groups);
+    return $groups;
...
+  public function clearCachedDefinitions() {
+    foreach ($this->getGroups() as $group_name => $label) {
+      $this->cacheBackend->delete($this->cacheKey . ':' . $group_name);
+    }
+    $this->cacheBackend->delete($this->cacheKey . '::groups');
+    parent::clearCachedDefinitions();

It's not clear why this needed because all caches are cleared when extension is installed

Also seems manager forgets to clear it static $breakpointsByGroup (that needs docs)

Also it looks strange that cacheKey uses language

@andypost

When I enabled a theme, the cache was not cleared. I assumed the same would be true for modules.

@Jelle_S nice work! Especially the theme disable cache clear!

Patch attached:

• moves some of the provider logic to BreakpointManager from ResponsiveImageMapping::calculateDependencies - the config entity shouldn't know too much about breakpoints :)
• moves stuff around in breakpoint to be consistent
• adds to the unit test for the responsive image mapping configuration entity class
• Plugin cache does not need to be cleared for modules this is handled automatically during module uninstall - themes are different (unfortunately)
• Uses cache tags instead of a custom clearCachedDefinitions method

Working on an issue summary update.

@dcrocks steps to reproduce your problem would be awesome.

@andypost language is part of the cache key because some of the breakpoint definition is translatable - ie the label.

The diff is to #75 since I was working on this concurrently to @Jelle_S - I've merged in the improvements from #81 and #82

I found only a absence of some docs.
@alexpott I'm not sure that label is used somewhere... but it could use TranslationWrapper for, at least I found no services that uses this pattern for cache key

Also https://www.drupal.org/documentation/modules/breakpoint will need update

1. +++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
   @@ -664,4 +664,34 @@ protected function systemThemeList() {
   +  public function themeExists($theme) {
   
   +++ b/core/lib/Drupal/Core/Extension/ThemeHandlerInterface.php
   @@ -131,4 +131,39 @@ public function getBaseThemes(array $themes, $theme);
   +   * Determines whether a given theme is enabled.
   ...
   +  public function themeExists($theme);
   

   isThemeEnabled() looks better, exists is wrong in that context

2. +++ b/core/modules/breakpoint/src/BreakpointManager.php
   @@ -0,0 +1,217 @@
   +  /**
   +   * @var array
   +   */
   +  protected $breakpointsByGroup;
   ...
   +    $this->breakpointsByGroup = NULL;
   

   var needs docs

3. +++ b/core/modules/breakpoint/src/BreakpointManager.php
   @@ -0,0 +1,217 @@
   +  /**
   +   * @param string $group
   +   *
   +   * @return string
   +   *   The label
   +   */
   +  protected function getGroupLabel($group) {
   

   doc block needs clean-up

4. +++ b/core/themes/stark/stark.breakpoints.yml
   @@ -0,0 +1,18 @@
   \ No newline at end of file
   

   :)

@andypost this is copied pretty much verbatim from contextual links manager

1. This is a copy of the moduleHandler function - at some point I'd like ThemeHandler and ModuleHandler to be more consistent.
2. Fixed
3. Fixed
4. Fixed

test fails in #87 look like bot issues.

Steps are pretty simple.
Make new clone of d8.0
Install
Enable Responsive_image module
Go to admin/config/media/responsive image mappings
Add a new mapping using the bartik breakpoints
Get success msg, but mapping not displayed. Clear caache multiple times with no affect.
I have provided images of the responsive image page after I saved the mapping, the message log showing it was saved, and a snip if the database showing the record for the mapping.

This actually looks like a problem I reported a while back(#2267563: Uncaught SQLite failure breaks responsive images).

#91 I tried it on a clean checkout and the patch from #90 and it works like it should.

Tried with patch in #90. Still a problem for me. Drupal seems to have a problem with being installed on VirtualDocumentRoot instead of DocumentRoot and has since alpha 10. But my problem shouldn't hold this up.

ps. I am using a different mac now and a different version of OS X, but all my d8 problems persist.

@dcrocks you may want to revisit your issue #2260969: 404 error on css/js aggregated files in site located in a VirtualDocumentRoot as most of the problems you are seeing stem from that eh?

Patch no longer applies.

Rerolled

Updated issue summary to reflect current solution.

When this is committed https://www.drupal.org/node/1813914 needs to be completely rewritten - it is hopelessly out of date with HEAD anyway :)

Patch in #2260061-74: Responsive image module does not support sizes/picture polyfill 2.2 has already been created assuming this patch will be committed. This way we can move on a bit faster with the Picturefill issue.

Sigh. Had a much longer review and Safari's stupid "oh, you moved your mouse, OBVIOUSLY that means you meant to go back a page and obliterate your Dreditor review" behavior killed it. :( Mostly it was a bunch of nitpicks, tho.

The bigger points were:

- WOW! this patch is amazing!!! I can't get over this diffstat: 66 files changed, 1136 insertions, 2262 deletions.
- How is "label" going to get translated, if this is not running though CMI or other standard system? If it can't be translated, can we ditch it, in favor of the YAML key being the label, if that's ever exposed anywhere?
- (not for this patch/issue, pre-existing condition) The tests could really do with some comments explaining what they're testing and why.

Pausing on committing due to the label question.

We need the label, it is used in the picture mapping, so yes it has to be translatable.

Do you want it as part of this patch, or will a follow up works as well?

Okay so this means we have to instantiate plugins - so we can get a translated label. This turns out to be great because now responsive image has completely no idea of the internal workings of the breakpoint module - it is just using methods on the breakpoint plugin and manager interfaces - yay!

Also

• added more complete testing of the sorting of breakpoints by weight
• tried to sort out translating groups - this needs review I think we I've done will work but this is complex because module and theme names should not be translated
• injected the breakpoint manager where possible in the responsive image module

Also we need to make D8MI aware of this issue because it affects things like #2322839: Unify YAML translation extraction, and allow extension by contrib.

Please open an issue in the potx module's queue and tag it with "Drupal 8 compatibility"

#114 Done, see #2325321: Support for Drupal 8 Breakpoint

Did a quick scan over the patch and found a small issue in BreakpointManager::getGroups():

+++ b/core/modules/breakpoint/src/BreakpointManager.php
@@ -0,0 +1,274 @@
+        if (!isset($groups[$plugin_definition['group']])) {
+          $groups[] = $plugin_definition['group'];
+        }

Should be (forgot the array key):

if (!isset($groups[$plugin_definition['group']]) {
  $groups[$plugin_definition['group']] = $plugin_definition['group'];
}

A more extensive review is still necessary though.

We need the label, it is used in the picture mapping, so yes it has to be translatable.

I found no usage after patch so open this question, only group name is used (or seems this is a extension name)

@attiks please point the loc of label usage! looks it's not needed at all

#123 It is used at http://cgit.drupalcode.org/drupal/tree/core/modules/responsive_image/src... which is altered by this patch to use the label. $label = $multiplier . ' ' . $breakpoint->getLabel() . ' [' . $breakpoint->getMediaQuery() . ']';

For clarity, the label is something like 'wide', which should be translatable. If we use the key (seven.wide) it will look strange

Example

seven.wide:
  label: wide

+++ b/core/modules/responsive_image/src/ResponsiveImageMappingForm.php
@@ -67,23 +92,23 @@ public function form(array $form, FormStateInterface $form_state) {
+        $label = $multiplier . ' ' . $breakpoint->getLabel() . ' [' . $breakpoint->getMediaQuery() . ']';
+        $form['keyed_mappings'][$breakpoint_id][$multiplier] = array(
...
+          '#title' => $label,

yep, it's really questionable how to translate this kind of strings

Why not just label it Wide viewports?

How it looks:

Rerolledl

Is this ending up in *.info.yml files or the title / summary need updating? Looking at the patch it uses its own plugin YAML format?

Forgot to fix the toolbar module to use the new breakpoint methods.

Did a review, most of them are comment nitpicks really.

1. +++ b/core/lib/Drupal/Core/Extension/ThemeHandlerInterface.php
   @@ -139,4 +139,39 @@ public function getName($theme);
   +   * @param string $theme
   +   *   The name of the theme (without the .module extension).
   

   "without the .theme extension" ?

2. +++ b/core/modules/breakpoint/src/BreakpointManager.php
   @@ -0,0 +1,274 @@
   +      foreach ($this->getDefinitions() as $plugin_definition) {
   +        if (!isset($groups[$plugin_definition['group']])) {
   +          $groups[] = $plugin_definition['group'];
   +        }
   +      }
   

   see #116

3. +++ b/core/modules/breakpoint/src/BreakpointManager.php
   @@ -0,0 +1,274 @@
   +  /**
   +   * Gets the label for a breakpoint group.
   +   *
   +   * @param string $group
   +   *   The breakpoint group.
   +   *
   +   * @return string
   +   *   The label.
   +   */
   +  protected function getGroupLabel($group) {
   +    // Extension names are not translatable.
   +    if ($this->moduleHandler->moduleExists($group)) {
   +      $label = $this->moduleHandler->getName($group);
   +    }
   +    elseif ($this->themeHandler->themeExists($group)) {
   +      $label = $this->themeHandler->getName($group);
   +    }
   +    else {
   +      // Custom group label that should be translatable.
   +      $label = $this->t($group, array(), array('context' => 'breakpoint'));
   +    }
   +    return $label;
   +  }
   

   Is there a reason why this is not part of the BreakpointManagerInterface?

4. +++ b/core/modules/breakpoint/tests/src/BreakpointTest.php
   @@ -0,0 +1,122 @@
   +/**
   + * @file
   + * Contains \Drupal\breakpoint\Tests\BreakpointTest
   + */
   

   Punctuation.

5. +++ b/core/modules/breakpoint/tests/src/BreakpointTest.php
   @@ -0,0 +1,122 @@
   +  /**
   +   * Setups the local task default.
   +   */
   

   "Sets up the breakpoint defaults" ?

Is there a reason why this is not part of the BreakpointManagerInterface?

protected methods cannot be on interfaces, only public methods.

New patch with remarks form #135 fixed.

Awesome, my feedback has been addressed. This is definitely going to be much easier for themers. Great work.

Committed and pushed to 8.x. Thanks!!!

If anyone is still paying attention, it appears my problem is a SQLite issue. I broke down and installed mysql on my mac and installed D8 with that. Everything appears to work fine on this system.

Patch #2318191: [meta] Database tests fail on SQLite corrects the failing behavior I have been seeing. Still, not exactly sure why.

Awesome, thanks dcrocks!