vikunja/frontend
Archived
6
39
Fork 48
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feature/move-styles-to-components #874

Merged
konrad merged 45 commits from dpschen/frontend:feature/move-styles-to-components into main 2021-10-21 19:11:18 +00:00
Conversation 14 Commits 45 Files Changed 105 +67 -65
18 changed files with 67 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit ad1f69a817 - Show all commits

4
src/App.vue
View File

@ -117,3 +117,7 @@ export default defineComponent({
},
})
</script>
<style lang="scss">
konrad marked this conversation as resolved
konrad commented 2021-10-19 19:05:52 +00:00
Review
Copy Link

Did you put this seperately from the other style tag to prevent importing and prefixing these global styles in the scope? (possibly multiple times)

Did you put this seperately from the other style tag to prevent importing and prefixing these global styles in the scope? (possibly multiple times)
dpschen commented 2021-10-19 22:30:50 +00:00
Review
Copy Link

Not sure if I 100% understand the question. But I'll just try to explain why there are two style tag in this component:

  1. the first (without scoped attribute) contains all global styles. The are just loaded here for the whole application.
    Since they are global the CSS rules inside also apply to all other components. Our target is to minimize the content of this file.
    But some will always stay there. E.g. CSS resets, typo imports.

  2. The style with the scoped attribute contains the styles that get scoped by the vue loader (vue-loader was in vue 2) to the current compoent via the css data attributes. I guess we could also abstract the offline stuff in its own component then these styles would be there together with the template.

When compiled by the vue-loader both of these kind of styles will get the
compiled output of the scss defined in the additionalData in the vite.config.js which should only be variables and mixins (so it should not result in rendered CSS code). With this "trick" (I think its kind of the way to go for vue) every component has all the global variables available. In reality every component imports them again on their own.

In general I tried to change as few as possible with the styles. But for example below we could just unindent all styles and decrease the complexitiy by decreasing the specifity:

.offline {
  background: url('@/assets/llama-nightscape.jpg') no-repeat center;
  background-size: cover;
  height: 100vh;
}

.offline-message {
  text-align: center;
  position: absolute;
  width: 100vw;
  bottom: 5vh;
  color: $white;
  padding: 0 1rem;
}

h1 {
  font-weight: bold;
  font-size: 1.5rem;
  text-align: center;
  color: $white;
  font-weight: 700 !important;
  font-size: 1.5rem;
}

Even if there is an <h1> somewhere else in the app: just the one in this component should be styles by this rule because of the scoping =)

Almost every component in this pull request could be unindented in this way.

The font-weight: 700 !important; is a topic in itself :)

Not sure if I 100% understand the question. But I'll just try to explain why there are two style tag in this component: 1) the first (without `scoped` attribute) contains _all_ global styles. The are just loaded here for the whole application. Since they are global the CSS rules inside also apply to all other components. Our target is to minimize the content of this file. But some will always stay there. E.g. CSS resets, typo imports. 2) The style with the `scoped` attribute contains the styles that get scoped ~~by the vue loader~~ (vue-loader was in vue 2) to the current compoent via the css data attributes. I guess we could also abstract the offline stuff in its own component then these styles would be there together with the template. When compiled ~~by the vue-loader~~ both of these kind of styles will get the compiled output of the scss defined in the `additionalData` in the vite.config.js which should only be variables and mixins (so it should _not_ result in rendered CSS code). With this "trick" (I think its kind of the way to go for vue) every component has all the global variables available. In reality every component imports them again on their own. In general I tried to change as few as possible with the styles. But for example below we could just unindent all styles and decrease the complexitiy by decreasing the [specifity](https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity): ```scss .offline { background: url('@/assets/llama-nightscape.jpg') no-repeat center; background-size: cover; height: 100vh; } .offline-message { text-align: center; position: absolute; width: 100vw; bottom: 5vh; color: $white; padding: 0 1rem; } h1 { font-weight: bold; font-size: 1.5rem; text-align: center; color: $white; font-weight: 700 !important; font-size: 1.5rem; } ``` Even if there is an `<h1>` somewhere else in the app: just the one in this component should be styles by this rule because of the scoping =) Almost every component in this pull request could be unindented in this way. The `font-weight: 700 !important;` is a topic in itself :)
konrad commented 2021-10-21 18:54:27 +00:00
Review
Copy Link

Thanks for clarifying!

In reality every component imports them again on their own.

Doesn't that bloat all component stylesheets because they all contain all global stylesheets? Or is vite smart enough to figure that out and create small bundles?

In general I tried to change as few as possible with the styles. But for example below we could just unindent all styles and decrease the complexitiy by decreasing the specifity:

We should clean the stylesheets up at some point. They've all grown over time ("historisch gewachsen") 🙂 I think that's better for another PR though because this one is already quite big.

Thanks for clarifying! > In reality every component imports them again on their own. Doesn't that bloat all component stylesheets because they all contain all global stylesheets? Or is vite smart enough to figure that out and create small bundles? > In general I tried to change as few as possible with the styles. But for example below we could just unindent all styles and decrease the complexitiy by decreasing the specifity: We should clean the stylesheets up at some point. They've all grown over time ("historisch gewachsen") 🙂 I think that's better for another PR though because this one is already quite big.
@import '@/styles/global.scss';
</style>

2
src/main.ts
View File

@ -23,8 +23,6 @@ import {formatDate, formatDateShort, formatDateLong, formatDateSince} from '@/he
// @ts-ignore
import {VERSION} from './version.json'
// Add CSS
import './styles/vikunja.scss'
// Notifications
import Notifications from '@kyvg/vue3-notification'

0
src/styles/utilities/animations.scss → src/styles/animations.scss
View File

25
src/styles/components/_all.scss
View File

@ -1,25 +0,0 @@
@import 'base/all';
@import 'attachments';
@import 'gantt';
@import 'labels';
@import 'list';
@import 'reminders';
@import 'switch-view';
@import 'task';
@import 'taskRelations';
@import 'tasks';
@import 'teams';
@import 'migrator';
@import 'comments';
@import 'table-view';
@import 'kanban';
@import 'list-backgrounds';
@import 'color-picker';
@import 'namespaces';
@import 'legal';
@import 'keyboard-shortcuts';
@import 'api-config';
@import 'datepicker';
@import 'notifications';
@import 'quick-actions';

26
src/styles/components/_index.scss Normal file
View File

@ -0,0 +1,26 @@
@import "base";
@import "attachments";
@import "gantt";
@import "tooltip";
@import "labels";
@import "list";
@import "reminders";
@import "switch-view";
@import "task";
@import "taskRelations";
@import "tasks";
@import "teams";
@import "migrator";
@import "comments";
@import "table-view";
@import "kanban";
@import "list-backgrounds";
@import "color-picker";
@import "namespaces";
@import "legal";
@import "keyboard-shortcuts";
@import "api-config";
@import "datepicker";
@import "notifications";
@import "quick-actions";

3
src/styles/components/base/_all.scss → src/styles/components/base/_index.scss
View File

@ -1,5 +1,4 @@
@import 'card';
@import 'fancycheckbox';
@import 'multiselect';
@import 'scrollbars';
@import 'tooltip';
@import 'scrollbars';

0
src/styles/components/base/tooltip.scss → src/styles/components/tooltip.scss
View File

0
src/styles/theme/fonts.scss → src/styles/fonts.scss
View File

18
src/styles/global.scss Normal file
View File

@ -0,0 +1,18 @@
@import "fonts";
@import "transitions";
@import "animations";
// This imports are the same as in "bulma/bulma.sass"
// with the expeption of the bulma utilities.
// They are imported globally in variables.scss
@import "bulma/sass/base/_all";
@import "bulma/sass/elements/_all";
@import "bulma/sass/form/_all";
@import "bulma/sass/components/_all";
@import "bulma/sass/grid/_all";
@import "bulma/sass/helpers/_all";
@import "bulma/sass/layout/_all";
@import "theme";
@import "components";

11
src/styles/theme/_all.scss
View File

@ -1,11 +0,0 @@
@import 'theme';
@import 'content';
@import 'form';
@import 'link-share';
@import 'loading';
@import 'navigation';
@import 'notification';
@import 'offline';
@import 'update-notification';
@import 'background';

11
src/styles/theme/_index.scss Normal file
View File

@ -0,0 +1,11 @@
@import "theme";
@import "content";
@import "form";
@import "link-share";
@import "loading";
@import "navigation";
@import "notification";
@import "offline";
@import "update-notification";
@import "background";

15
src/styles/theme/theme.scss
View File

@ -1,18 +1,3 @@
// bulma utilities are imported in variables.scss
@import "bulma/sass/utilities/_all";
@import "bulma/sass/base/_all";
@import "bulma/sass/elements/_all";
@import "bulma/sass/form/_all";
@import "bulma/sass/components/_all";
@import "bulma/sass/grid/_all";
@import "bulma/sass/helpers/_all";
@import "bulma/sass/layout/_all";
@import "fonts";
@import "variables-derived";
*,
*:hover,
*:active,

2
src/styles/theme/variables-derived.scss
View File

@ -1,2 +0,0 @@
// Variables that are derived from bulma variables need to be included after them
$mobile: math.div($tablet, 2);

0
src/styles/utilities/transitions.scss → src/styles/transitions.scss
View File

2
src/styles/utilities/_all.scss
View File

@ -1,2 +0,0 @@
@import 'animations';
@import 'transitions';

8
src/styles/variables/_index.scss
View File

@ -3,4 +3,10 @@
@import "variables";
// the default values get overwritten by the definitions above
@import "bulma/sass/utilities/_all";
@import "bulma/sass/utilities/_all";
// this is needed so that the shared form variables are globally defined aswell
@import "bulma/sass/form/shared";
// since $tablet is defined by bulma we can just define it after importing the utilities
$mobile: math.div($tablet, 2);

2
src/styles/variables/variables.scss
View File

@ -1,5 +1,3 @@
@import 'colors';
$white: #fff;
$black: hsl(0, 0%, 4%) !default;
$orange: #ff851b;

3
src/styles/vikunja.scss
View File

@ -1,3 +0,0 @@
@import 'theme/all';
@import 'utilities/all';
@import 'components/all';