Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfc: new layout system #1185

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,29 @@
"postinstall": "ngcc --properties es2015 browser module main --create-ivy-entry-points"
},
"version": "9.0.0-beta.29",
"requiredAngularVersion": ">=9.0.0-rc.11",
"requiredAngularVersion": ">=9.0.0",
"dependencies": {
"@angular/cdk": "^9.0.0-rc.8",
"@angular/common": "^9.0.0-rc.11",
"@angular/compiler": "^9.0.0-rc.11",
"@angular/core": "^9.0.0-rc.11",
"@angular/platform-browser": "^9.0.0-rc.11",
"@angular/cdk": "^9.0.0",
"@angular/common": "^9.0.0",
"@angular/compiler": "^9.0.0",
"@angular/core": "^9.0.0",
"@angular/platform-browser": "^9.0.0",
"core-js": "^2.5.7",
"karma-parallel": "^0.3.1",
"rxjs": "^6.5.1",
"systemjs": "0.19.43",
"tsickle": "^0.37.1",
"tsickle": "^0.38.0",
"tslib": "^1.9.3",
"zone.js": "~0.10.2"
},
"devDependencies": {
"@angular/animations": "^9.0.0-rc.11",
"@angular/compiler-cli": "^9.0.0-rc.11",
"@angular/forms": "^9.0.0-rc.11",
"@angular/material": "^9.0.0-rc.8",
"@angular/platform-browser-dynamic": "^9.0.0-rc.11",
"@angular/platform-server": "^9.0.0-rc.11",
"@angular/router": "^9.0.0-rc.11",
"@angular/animations": "^9.0.0",
"@angular/compiler-cli": "^9.0.0",
"@angular/forms": "^9.0.0",
"@angular/material": "^9.0.0",
"@angular/platform-browser-dynamic": "^9.0.0",
"@angular/platform-server": "^9.0.0",
"@angular/router": "^9.0.0",
"@firebase/app-types": "^0.3.2",
"@types/chalk": "^0.4.31",
"@types/fs-extra": "^4.0.5",
Expand Down
55 changes: 55 additions & 0 deletions src/lib/uni/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
The `uni` entrypoint is a completely new approach to the Layout library. The emphasis here
is on simplicity, with as little defined in the library as possible. The goal is to create
a utility around the hardest part of layout management: creating and defining interchangeable
builders and coordinating that with the media query system in the browser.

This is a backwards-incompatible approach to the current version of Angular Flex Layout, but
we believe that this approach is more ergonomic in nature, and is the right way of doing things
moving forward. That being said, this is completely opt-in, with no actual plans to transition
anywhere in the near- or long-term.

To use this is quite simple:

### Step 1: Import the new module
```ts
// app.module.ts

@NgModule({
imports: [UnifiedModule.withDefaults()] // brings in default tags and breakpoints
})
export class AppModule {}
```

### Step 2: Use the new syntax
```html
<div ngl flexAlign="start">
<bp tag="xs" flexAlign="end"></bp>
<bp tag="md" flexAlign="center"></bp>
Comment on lines +26 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description uses name="xs" here. It looks like the rest of the code uses tag="xs".

I'm not sure which one is clearer, probably name?

I guess the formal name for "the stuff inside the media query CSS at-rule" is a "media query list". But I don't think that we support multiple queries per <bp>. Perhaps it should just be "query="xs"?

</div>
```

Here, `ngl` stands for "Angular (ng) Layout (l)". The `bp` elements are optional if
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was ngl selected just because it's shorter than ngLayout?

you don't plan to use breakpoints in your template. Conceptually, it's cleaner than
cramming all of the breakpoints on one root element, and the `bp` elements don't
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bp is short, but I think that it lacks a little bit of expressiveness. It won't be readable to people who aren't familiar with this library's APIs.

Is <breakpoint overly verbose? It seems a lot more readable and expressive.

<div ngLayout flexAlign="start">
  <breakpoint tag="xs" flexAlign="end"></breakpoint>
  <breakpoint tag="md" flexAlign="center"></breakpoint>
</div>

actually render in the template anyway.

We've removed the `fx` and `gd` prefixes, since now we can rely on attributes that
are only applicable on the `bp` and root `ngl` elements.

Again, our goal here is simplicity. The most basic usage could be construed as the
following:

```html
<div ngl flex></div>
```

This roughly corresponds to the old syntax for accomplishing the same:

```html
<div fxFlex></div>
```

While users who don't use many attributes per directive may not see a difference,
it will become quite apparent to those who use many, or as you scale.

This is only a proposal, and all feedback is welcome.
9 changes: 9 additions & 0 deletions src/lib/uni/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

export * from './public-api';
49 changes: 49 additions & 0 deletions src/lib/uni/module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {ModuleWithProviders, NgModule} from '@angular/core';

import {BreakpointDirective, UnifiedDirective} from './src/unified';
import {FLEX_PROVIDER, GRID_PROVIDER, TAGS_PROVIDER} from './src/tags/tags';
import {BREAKPOINTS_PROVIDER} from './src/breakpoint';


@NgModule({
declarations: [UnifiedDirective, BreakpointDirective],
exports: [UnifiedDirective, BreakpointDirective]
})
export class UnifiedModule {
static withDefaults(withDefaultBp: boolean = true): ModuleWithProviders<UnifiedModule> {
return {
ngModule: UnifiedModule,
providers: withDefaultBp ? [
TAGS_PROVIDER,
BREAKPOINTS_PROVIDER,
] : [TAGS_PROVIDER],
};
}

static withFlex(withDefaultBp: boolean = true): ModuleWithProviders<UnifiedModule> {
return {
ngModule: UnifiedModule,
providers: withDefaultBp ? [
FLEX_PROVIDER,
BREAKPOINTS_PROVIDER
] : [FLEX_PROVIDER]
};
}

static withGrid(withDefaultBp: boolean = true): ModuleWithProviders<UnifiedModule> {
return {
ngModule: UnifiedModule,
providers: withDefaultBp ? [
GRID_PROVIDER,
BREAKPOINTS_PROVIDER
] : [GRID_PROVIDER]
};
}
}
10 changes: 10 additions & 0 deletions src/lib/uni/public-api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

export * from './src/uni';
export * from './module';
145 changes: 145 additions & 0 deletions src/lib/uni/src/breakpoint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {inject, InjectionToken} from '@angular/core';


/**
* A breakpoint is a wrapper interface around the browser's mediaQuery,
* which is a condition string used for matching based on browser window
* parameters. Here, a breakpoint has a shortcut name, e.g. 'xs', the
* corresponding mediaQuery, and a priority in case the mediaQuery overlaps
* with other registered breakpoints.
*
* @see https://developer.mozilla.org/en-US/docs/Web/CSS/Media_Queries/Using_media_queries
*/
export interface Breakpoint {
/** The shortcut name for the breakpoint, e.g. 'xs' */
name: string;
/** The mediaQuery for the breakpoint, e.g. 'screen and (max-width: 500px)' */
media: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that there's a good case for calling this query or mediaQuery.

/** The priority of the breakpoint compared to other breakpoints */
priority: number;
}

export const FALLBACK_BREAKPOINT_KEY: string = '__FALLBACK__';

/**
* The fallback breakpoint, which has no real name and is
* superseded by any other breakpoint value
*/
export const FALLBACK_BREAKPOINT: Breakpoint = {
name: FALLBACK_BREAKPOINT_KEY,
media: 'all',
priority: -Number.MAX_SAFE_INTEGER,
};

/**
* The default breakpoints as provided by Google's Material Design.
* These do not include orientation breakpoints or device breakpoints.
*/
export const DEFAULT_BREAKPOINTS: Breakpoint[] = [
{
name: 'xs',
media: 'screen and (min-width: 0px) and (max-width: 599.9px)',
priority: 1000,
},
{
name: 'sm',
media: 'screen and (min-width: 600px) and (max-width: 959.9px)',
priority: 900,
},
{
name: 'md',
media: 'screen and (min-width: 960px) and (max-width: 1279.9px)',
priority: 800,
},
{
name: 'lg',
media: 'screen and (min-width: 1280px) and (max-width: 1919.9px)',
priority: 700,
},
{
name: 'xl',
media: 'screen and (min-width: 1920px) and (max-width: 4999.9px)',
priority: 600,
},
{
name: 'lt-sm',
media: 'screen and (max-width: 599.9px)',
priority: 950,
},
{
name: 'lt-md',
media: 'screen and (max-width: 959.9px)',
priority: 850,
},
{
name: 'lt-lg',
media: 'screen and (max-width: 1279.9px)',
priority: 750,
},
{
name: 'lt-xl',
priority: 650,
media: 'screen and (max-width: 1919.9px)',
},
{
name: 'gt-xs',
media: 'screen and (min-width: 600px)',
priority: -950,
},
{
name: 'gt-sm',
media: 'screen and (min-width: 960px)',
priority: -850,
}, {
name: 'gt-md',
media: 'screen and (min-width: 1280px)',
priority: -750,
},
{
name: 'gt-lg',
media: 'screen and (min-width: 1920px)',
priority: -650,
}
];

/**
* The user-facing injection token for providing breakpoints,
* this is meant to be provided as a multi-provider, and
* consolidated later.
*/
export const BREAKPOINTS =
new InjectionToken<Array<Array<Breakpoint>>>('Angular Layout Breakpoints');

/** An internal-facing provider for the default breakpoints */
export const BREAKPOINTS_PROVIDER = {
provide: BREAKPOINTS,
useValue: DEFAULT_BREAKPOINTS,
multi: true,
};

/**
* An internal-facing injection token to consolidate all registered
* breakpoints for use in the application.
*/
export const BPS = new InjectionToken<Breakpoint[]>('Angular Layout Condensed Breakpoints', {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a short token, but it's not very expressive.

providedIn: 'root',
factory: () => {
const providedBps = inject(BREAKPOINTS);
const bpMap: Map<string, Breakpoint> = new Map();

providedBps.forEach(bps => {
bps.forEach(bp => {
bpMap.set(bp.name, bp);
});
});

return [...Array.from(bpMap.values()), FALLBACK_BREAKPOINT];
}
});
Loading