feat: expose types - Githubissues

zunnzunn / vue-ganttastic

Simple, interactive and highly customizable Gantt chart component for Vue 3

https://zunnzunn.github.io/vue-ganttastic/

566 stars 141 forks source link

feat: expose types #53

Closed dpschen closed 2 years ago

dpschen commented 2 years ago

I realised that this library uses types but they were not exposed. By trying to fix this I went a bit overboard and did the following:

expose types for library users
upgrade packages and vuepress
use vite for bundling
provide context with new composable 'useContext'
use prettier to format
fix minor type issues
styles:
- remove style scoping, to make overwriting styles easier
- remove usage of ids (in case you want to use the chart multiple times)
- prefix animations
- inherit font by default, since Helvetica isn't available on every system
- use ew-resize cursor to show that resizing is possible in two directions and align with common ui pattern
use correct typographic character for range values
use numeric font variant so that dates always require the same width
align wording 'getRowsInChart' and 'getChartRows'

Somewhere along the way I broke something minor (the rendering is a bit broken right now). If there is general interest in merging these or some of these changes I would try to look into what is creating the issue.

TODO:

[x] use postcss-preset-env to remove the need to manually write vendor prefixes
[x] fix newly introduced rendering bug

zunnzunn commented 2 years ago

Hello! Yes I'd be interested in merging this. Could you just clarify the following point:

use numeric font variant so that dates always require the same width

dpschen commented 2 years ago

Hello! Yes I'd be interested in merging this. Could you just clarify the following point:

use numeric font variant so that dates always require the same width

Sure! This is regarding https://github.com/dpschen/vue-ganttastic/blob/feature/expose-types/src/components/GGanttChart.vue#L249. By defining font-variant-numeric: tabular-nums; we use the OpenType feature tnum if the used font supports it. This makes sure that numbers always have the same width (aka optimised for tables). I added this because the time preview in the tooltip 'jumps' around when dragging the start or end of a bar.

These screens are not perfectly cut but maybe you can see the different width here in the minute numbers of the start time (11 vs 59). It's better visible when it is in motion while you drag it around.

SCR-20220902-sw5 SCR-20220902-svi

zunnzunn commented 2 years ago

Hello! Yes I'd be interested in merging this. Could you just clarify the following point:

use numeric font variant so that dates always require the same width

Sure! This is regarding https://github.com/dpschen/vue-ganttastic/blob/feature/expose-types/src/components/GGanttChart.vue#L249. By defining font-variant-numeric: tabular-nums; we use the OpenType feature tnum if the used font supports it. This makes sure that numbers always have the same width (aka optimised for tables). I added this because the time preview in the tooltip 'jumps' around when dragging the start or end of a bar.

These screens are not perfectly cut but maybe you can see the different width here in the minute numbers of the start time (11 vs 59). It's better visible when it is in motion while you drag it around.

Excellent! Thumbs up from me - tell me when you fix the broken rendering and I will review this pull request :smile:

dpschen commented 2 years ago

@InfectoOne: If you have time you can give it a try :)

zunnzunn commented 2 years ago

Done! Thank you for your contribution, @dpschen !

zunnzunn commented 2 years ago

I will create a new build based on this version and publish it to npm. Btw. I have noticed that you have bumped the version to 3.0.0. It'd be better to bump it from 2.0.4 to 2.1.0, since it's not a major release, but a minor release with a new minor functionality (the types are now exposed) and a lot of patches/fixes. Thoughts on this, @dpschen ?

dpschen commented 2 years ago

@InfectoOne

Nice to hear! I was not sure if I changed the API at some point in a wake that brakes stuff, so I better wanted to make sure to indicate bigger changes. The 'css-api' did change though. So some manual style overwrites might break.

I didn't fix the naming alignment now, because I was waiting for your answer.

zunnzunn commented 1 year ago

No problem, I will fix the naming alignment myself. Technically speaking, the changes made to the CSS styling are 'breaking changes' to some extent, but manual CSS overwrites are often a "dirty/hacky" matter which if (slightly) broken does not justify a major release (in my opinion), so the version will be bumped to 2.1.0.