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

With styling #17

Merged
merged 18 commits into from
Aug 13, 2020
Merged

With styling #17

merged 18 commits into from
Aug 13, 2020

Conversation

Haprog
Copy link
Contributor

@Haprog Haprog commented Aug 7, 2020

Fixes #8

Viktor Lukashov and others added 13 commits July 14, 2020 13:57
This implements the UX requirements listed in https://docs.google.com/document/d/1xftbmiAw491k0uag6lJi46B7slidXfmvOMbOgKm3G8U:
 - accessing any view requires logging in
 - accessing any endpoint requires logging in
 - deep links work even if logging in is required
 - expired sessions are re-established without loosing the app state

The DX needs improvement. See the TODOs in the code.
This allows importing LitElement css literals from .css files and using them directly.

For example, styles for the `login-view` component are imported from `login-view.css`:
```ts
import styles from "./login-view.css";

@CustomElement('login-view')
export class LoginView extends LitElement {
  static styles = styles;
}
```

instead of including them inline in the component code:

```ts
@CustomElement('login-view')
export class LoginView extends LitElement {
  static styles = css`
    [styles here]
  `;
}
```

NOTE: it requires a custom Webpack loader to avoid wrapping the imported CSS string into LitElement's `unsafeCSS` in the user code. Loader makes this wrapping automatic and invisible.

Ideally, the loader should be released by Vaadin to npm, and used in the generated webpack config file by default.
 - elevate the `src/views` folder up to `views` and rename it to `components`
 - create a separate folder for each component (to keep .css file next to the .ts file in the same folder)
 - gather should-not-be-needed and should-be-in-the-framework code inside a `utils` folder

This brings the project structure closer to what the starter wizard generates.
Conflicts:
	frontend/components/contact-form/contact-form.ts
	frontend/connect-client.ts
	frontend/index.ts
	frontend/src/views/contact-form.ts
Copy link
Contributor Author

@Haprog Haprog left a comment

Choose a reason for hiding this comment

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

This is my approving review for all the changes by Viktor in this branch. So far all I've done here is basically merged updates from with-security (which is now in master) and master and resolved all conflicts.

`lit-css-loader` is now used by default (in Flow) so we can remove customizations from webpack config.  Uses default contents for webpack.config.js now.

See vaadin/flow#8843
@Haprog Haprog marked this pull request as ready for review August 13, 2020 07:24
@Haprog
Copy link
Contributor Author

Haprog commented Aug 13, 2020

For making the review easier, I think it should be enough if someone else reviews only my changes to this branch, which is basically these 5 last commits since I've reviewed the older stuff.

And the current webpack.config.js is just a copy of the default one from flow-server/src/main/resources/webpack.config.js (to revert earlier customizations that are no longer needed).

@Haprog Haprog added the fusion label Aug 13, 2020
@Haprog Haprog removed the request for review from vlukashov August 13, 2020 12:05
@Haprog Haprog merged commit 4e3d212 into master Aug 13, 2020
@Haprog Haprog deleted the with-styling branch August 13, 2020 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use the Lumo theme consistently
3 participants