valantic / vue-template Goto Github PK

It seems, that the width and max-width values on the modal and the related computed are not needed. If you use CSS and give the modal margin: 0 auto; it should be possible to achieve the same result.

In recent projects we already used an outside click directive. This should be added to the vue-template as well, because it is required by almost every project.

When starting the dev environment, the name "faker" appears quite a lot while the modules are loaded. This should be improved. Maybe by limiting the required locales? See latest Pimcore projects.

Currently e-select and e-multiselect require an array of objects with specific keys for identifier and value. This is not very convenient, since the data often needs to be mapped. The components would be easier to use if the source for the value and identifier could be defined by prop, so "any" array of object could be used as a source.

For IV we added a new SCSS mixin, which applies multiline line-clamping on modern browsers without JS.

I think it is time to take font-size handling one step further. Right now we always need to define the font-size for mobile and desktop. It would make more sense, if there was a definition of coupled font-sizeds. e.g. default font-size on mobile is 14px, on desktop 16px. Then, with a mixin those fonts then should be retrievable. The available font sets could be defined with a combination of variables and maps.

@include responsive-font($font-size--p);

$font-size--p: map-get($responsive-font-sizes, p);

$responsive-font-sizes: (
  "p": (
     "xxs": 14,
     "md": 16
  )
)

The moment.js library is quite big for what we use it. We should switch to Day.js which is much smaller to reduce the overall build size of the vendor.js.

https://github.com/iamkun/dayjs

It would be very helpful, if mocked requests would make a console log, so the calls and sent data can be checked.

Maybe this only happens in vue-styleguidist (Make sure you are in "single view mode" or use outsideclick, else the back to top will hide the error).

Across the template, there are several hardcoded line-heights used together with @include font(). They should be removed, to make search/replace easier at the beginning of a new project.

To overwrite some of the plugins styles, !important was used. It would be better to use a heavier selector instead, to get the same result (e.g. div.v--modal.

Notifications currently only have a type which holds information about the display type like error or info. We should define an additional property, that also defines, what kind of notification this is. e.g. Add-To-Cart message or global message or footer message, ...

So type would be about HOW a notification is displayed and the new property WHERE it is displayed.

When using mixins, the JSDoc is also shown in Styleguidist. This can be a bit confusing, because it is not clear where the prop is comming from. This could be improved by adding a prefix to the comments.

e.g. "form-states mixin Adds hover to state."

As in a discussion with @patric-eberle and @mob8607 we came to the conclusion that it might be better to use new settings for the attributes-order rule.

'vue/attributes-order': [2, {
      'order': [
        'DEFINITION',
        [
          'LIST_RENDERING',
          'CONDITIONALS',
          'RENDER_MODIFIERS',
          'OTHER_DIRECTIVES',
          'TWO_WAY_BINDING',
          'CONTENT'
        ], [
          'GLOBAL',
          'UNIQUE',
          'OTHER_ATTR',
        ],
        'EVENTS',
      ]
    }]

We should extract polyfills which are only relevant for one browser to reduce the vendor bundle size.

e.g.

• CSS Variables Ponyfill
• Picturefill

The used Vue plugin vue-js-modal seems quite big for the functionality we actually use.
https://bundlephobia.com/[email protected]

Some features like drag and drop or resizing we never used. I think it would be worth to create our own modal component, which implements the basics to reduce JS size, CSS complexity and makes us more flexible.

This would most likely deprecate #119 #120

Currently our i18n setup requires a manual definition of the available languages. This could be automated by extracting the available translation JSONs from the translation folder. This was implemented in IV.

e.g.

export const I18N_LOCALES = require.context('../translations/', false, /\.json/)
  .keys()
  .map(filePath => filePath.match(/(?<=\.\/).*?(?=\.json)/)[0]);

It would be helpful if we had a logic to focus the first field with an error in the document, where ever we currently are.

The used vue-tabs-component component is no longer actively maintained and should be removed or taken over into our own code.

The hasTouch mixin serves a computed property, that checks if the current device has touch support. But this test is misleading, since some devices may support touch, but still are used with a mouse (e.g. Windows). This mixing should be replaced with a logic, where on each click event it is evaluated if the event is in combination with a touch gesture.

Currently we only have project related translations. Therefore it's not possible to hold translations we only need in the styleguide (e.g. language selector).

We should add additional translation files which contain styleguide related translations and will be merged when working in dev mode.

It would help to have a state flag for each component in the styleguidist to recognize components which are not ready for production yet.

Currently api actions in Vuex are not distinguishable from other actions. We should add the prefix api for all actions, that communicate with the server. This makes the purpose of the actions clearer and also allows to identify server interactions directly in the components.

Currently, if you want to add a label to an icon you always have to add a separate element. I think we should add the possibility to add at least a hidden label without an additional element by using a prop or slot.

Setzen des Verhältnisses von Breite zu Höhe anhand der Ratio des Bildes.
Testen ob es mit sources und srcset funktioniert.

On the IV project, we replaced the body-scroll helper with a self maintained plugin, which is more reliable. Should be replaced here as well.

The stored state of a running request should hold not only the Boolean if the request is running. It should hold the request itself if it's running. So it's easier to handle cases where we need to get the specific request.

Example store module:

state: {
  runningRequests: {
    fetchData: null,
  },
},

...
getters: {
  getRequestIsRunning: state => Object.values(state.runningRequests).some(item => !!item),
}

...

mutations: {
  setRunningRequest(state, { id, request }) {
      state.runningRequests[id] = request;
  },
}

..

actions: {
  fetchData({ commit }) {
      const request = api.get('/get/the/data')
        .then((response) => {
          if (response) {
            return response;
          }

          return Promise.reject(new Error('API failure'));
        })
        .finally(() => {
          commit('setRunningRequest', { id: 'fetchData', request: null });
        });

      commit('setRunningRequest', { id: 'fetchData', request });

      return request;
  },
}

The notification setup contains already a lot of cases by default, of which some won't be used by future projects. The displayTypes should be reduced to one and types like add-to-cart should be removed.

Mostly we seem to use e-icon with the inline prop set to true. We should make this the default.

We have already several plugins like viewport and bem. These should also be documented in Styleguidist.

When images are lazy loaded, they are displayed quite harsh when eventually loaded. This could be improved by adding an onload transition effect. See IV as an example.

Currently the e-button component replaces it's content, when it switches to progress mode. This prevents us from using the v-t directive or any other directive, that places something inside the button, because it will be replaced with the progress. Would it instead be possible to use an absolute element for the progress state, that covers the main content? This would also prevent size changes when switching between the states.

The ajax promise should directly return the response data, since we have a convention about the response structure.

api.post().then((data, response) => {});

In IV we improved the v-outside-click directive to also allow a method as value instead of an object, because mostly no additional configuration is needed.

It would make sense to add an additional modifier for each cell, which represents the column name. This way it would be possible to style whole columns without the need of an additional cell template.

For IV we extended the focus-mask directive with the possibility to use variants. The transition was also improved.

The following props validator is used quite often with different scale numbers:

validator(value) {
          return [
            0,
            500
          ].includes(parseInt(value, 10));
}

It would be more convenient and reduce duplicated code if we had a constructor for this.

e.g.

props: {
   foo: {
      ...PROP_VALIDATION_SCALE([0, 200, 500]),
     type: Number,
     // ...
   }
}

Webpack version 4 has better tree shaking support and therefore should be able to create a smaller build output.

Currently the e-select component only returns the identifier of the currently selected element. It would be more convenient to receive the whole object.

Currently we use a lot of inline require(). Especially to import mock data in Vuex modules. To use inline (dynamic) import() would be better, since it is the native standard. Check if this is possible and if so, replace where possible.

Dynamic imports are currently in stage 2: https://github.com/tc39/proposals

Most element components have additional states and code, that was only required to apply the styling of the initial project. They should be reduced to the bare minimum, since it confuses external developers.

https://kazupon.github.io/vue-i18n/guide/directive.html#use-with-transitions

Inside a transition it looses the translation while the transition is running.
I would suggest, not to define it globally, but add it to the readme.md

The mockData in styleguide.options.js which was added for the styleguidist environment extends all components in dev mode. This is a bug and should be fixed. The mockData should only be added to the styleguidist Vue instance.

There are some uses of v-html which seem rather unnecessary, to have it as a default. e.g. when using c-form-notification.

v-html should be avoided where ever possible. Especially on this project template.

For IV we created two new components to handle date inputs. Since they contain no specific functionality (except for v-focus), it should be save to copy them into the boilerplate.

e-date

• Accepts a date string and returns a date object
• Date string format is based on the users sytem settings
• Can have an inline label and icon if required

c-date-picker

• Uses Pikaday
• Allows to select a single date or a range of dates

e-input

• Make sure internalValue gets updated when input value changes (watch)

Check if there is a better approach to simulate the initial data push. Currently, the initial data is imported directly in the vuex module. It would be better, if also in the dev environment, the initial data would be pushed to the store via the initial data actions, to test the import.

The form on http://localhost:8080/styleguide/sandbox/forms has some strange form examples (e.g. 1 label with multiple radios/checkboxes). It should be improved to pass the wave test. Since it is a good example for how to use forms, it should be kept though. Maybe adding validation would also be helpful.

Currently the demo navigation only allows one level of navigation items. Would be good if it would support multiple levels for visual grouping.

e.g.

- Sandbox
- Components
-- Component 1
- Pages
-- Homepage
--- Logged in
--- Logged out

The body-scroll helper in the boilerplate was replaced with a plugin in the meantime in W+F which added additional functionality. The helper should be replaced with this plugin.

Translations should not allow to contain HTML, because they can cause XXS. Also think of scenarios, where the translation itself could be converted into XXS critical code from outside, since the messages are available trough window.vm.

To prevent the use of HTML in translations warnHtmlInMessage should be turned on by default https://kazupon.github.io/vue-i18n/api/#constructor-options

To work with HTML and translations, the following approach(es) should be used: https://kazupon.github.io/vue-i18n/guide/interpolation.html#basic-usage