feat: show an optional background for the grid #81
Conversation
|
Nice work @arnoudb ! 👌 I added a commit to support 'rowHeight' = 'fit'. We could also simplify the solution, since both row height and column width are known and can be passed to the CSS variables, but is optional. About the things TODO, we need to find a better way to decide when the grid is showed or not. I see 2 options: 1- Show the grid background when is not null, and don't show it when null. I may prefer the solution number 1 for simplicity. Said that, If you have in mind other options we can discuss them. Let me know your thoughts about it. After this decision, we will be able to move forward on merging the full feature. |
|
Nice cleanup! Didn't realize you were going to commit on this branch as well though :-). We have now diverging branches as i implemented the 'whenDragging' and did some cleaning too. I'll rebase tomorrow. I updated the playground as well to allow for 'never'|'always'|'whenDragging' |
|
Yes, I can commit directly on this branch 😂. It is useful to help you with some commits, so you don't take all the work. Okay nice! you can solve the conflicts in this branch whatever you like, since before merging to 'main', you will have to squash all commits into a single 1. I have some doubts about the api with 'never'|'always'|'whenDragging'. I am not sure if adding another 'Input' only for the use case 'whenDragging' is appropriate, but if you have already done let's see. Also there is still 1 issue left to solve. This issue is related to the background rendering, see the image below:
If the border width is big enough is easy to see that the top and left borders are inside the grid item, and the bottom right borders are outside. I think the border of the rows and columns should go inside... but I am not 100% sure right now, since outside can make sense also. I will think about it. |
|
Yes, i understand you can! And of course help is welcome. I'm just used that pull request get comments and that the author fixes them 😄 About my code: I took the approach of not adding another @input. Its part of the config. I have the next logic:
I'll integrate your changes later today and re-commit. About the lines positions: I think i can come up with something. Basically shift the background to the left and top based on lineWidth. I would like it it to stick to whole pixels though. |
|
Oh, i fixed the grid position in the last commit as well. Hope you like the result. |
|
Thanks for you work @arnoudb ! 👏 About the grid position, I have finally opted to just show the border inside the rows and columns... It may not be the best in all cases, but seems solid and it sticks to whole pixels. Revise those commits, and if you agree with the additions/changes, for me is ready to merge 🚀 ! Before merging, you should squash all those 6 commits into a single 1, and name it something like: |
|
Cool, i like the changes! Really like how this thing is working out. Placing the lines inside and also have a gap color makes much sense! I'll double check everything tonight and then will squash the commits. |
arnoudb
force-pushed
the
grid-background-lines
branch
from
e3c2c7c
to
2264c21
Compare
|
@arnoudb Thanks for squashing the commits! Now is ready to merge :) Before doing it, I want to alert you that seems that your commit user is not linked with your actual github user. So you will not appear as a contributor. Here is a link that may help you fixing it. BTW If you don't care just tell me and I will merge straight away, but is always nice to have this issue fixed in my opnion. I will wait for your response! Thanks again! |
The user can now show a backround for the grid. It may be shown only whenDragging or always.
It can be configured by adding a configuration object to the grid:
KtdGridBackgroundCfg {
show: 'never' | 'always' | 'whenDragging';
borderColor?: string;
gapColor?: string;
rowColor?: string;
columnColor?: string;
borderWidth?: number;
}
set it like:
[backgroundConfig]="<your config object>"
Colors are provided as css color strings.
For an elaborate example see the playground.
arnoudb
force-pushed
the
grid-background-lines
branch
from
2264c21
to
d68b632
Compare
|
ah good one! changed the email and pushed again. |
Hi, created a proof of concept for showing the grid lines as a background using css.
Sure needs some cleanup but wanted to show you first. If you checkout the branch you can see in the playground how it works. Its quite configurable and you can pick different colors for lines, column background and row backgrounds.
Playing with transparency gives nice results.
Pls let me know what you think!
