Enable showing distribution on columns on index page

[bryszard]

Description

Fixes # 2162

The issue describes the feature in a general way, so I've applied it in my way, although according to our discussion on the Issue.

From the things worth mentioning - I've decided not to implement handling of multiple "widgets" on one column, eg. sortable and summarizable. In my opinion we can do it as a second iteration and separate task. Let me know if you thing otherwise @adrianthedev.

When it comes to the colors - I've noticed that although we are defining the default_chart_colors and chart_colors (on Avo::Configuration::Branding), we're not using it anywhere (or at least I haven't found the usage of it). For this reason, I've decided to change the default_chart_colors to match the choice of the chartkick lib that we're using. In their case you can find the defaults here and I think they have a valid reasoning behind the list.

It will be possible to customize the colors for charts, although in that case user will have to provide the arbitrary classes for the Tailwind (eg. in the safelist.txt).

• [x] I have performed a self-review of my own code
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [x] I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Updated after code review - 24.02.2024 (what a date!)

https://github.com/avo-hq/avo/assets/12682792/839c7ee8-787a-4683-98c3-ca0097db8536

Manual review steps

1. Run the dummy app and check the Project resource index page
2. Check the behavior when clicking on the bar icon near the "Status" table header

Manual reviewer: please leave a comment with output from the test if that's the case.

[codeclimate[bot]]

Code Climate has analyzed commit 25a8e3e9 and detected 0 issues on this pull request.

View more on Code Climate.

[bryszard]

I still need to add some specs and documentation.

[bryszard]

@adrianthedev The feature is done, added specs. I'll prepare documentation after the green light from the review.

[bryszard]

@adrianthedev Thank you for the review. I think I'll have some time to look on it on Sunday.

[bryszard]

@adrianthedev

Ad 2. I've removed the changes to the colors. As expected it doesn't look so good with the limited number of them:

Ad. 3. I've moved the generation of the bg- Tailwind classes to export-tailwind-safelist.js. However, I know that's not perfect, because we should have some way of using whatever is added to the configuration of colors. It's also not perfect, because even for the default ones, we have 2 sources of truth now (list of colors appears both in branding.rb and in export-tailwind-safelist.js. Do you have an idea how to keep the configuration and the JS script in sync?

Ad. 4. Ok, so I understand it is connected with your other comment, to use the turbo-frame? I didn't have time to do that yet, but if you confirm, I'll be happy to work on it :).

[adrianthedev]

2. After a rebase, they wll look better. https://github.com/avo-hq/avo/pull/2446 brings the new colors.
3. It's not a perfect, system, I know, but I don't expect to change them too often. (the new dashboard colors should be "#0B8AE2", "#34C683", "#FFBE4F", "#FF7676", "#2AB1EE", "#34C6A8", "#EC8CFF", "#80FF91", "#FFFC38", "#1BDBE8"
4. Cool. Thank you. Let me know if you need any help there.

[bryszard]

So I've added the turbo-frame with lazy-loading, but unfortunately I've got some things that do not work as expected:

1. I cannot make the lazy-loading work in the Capybara test. Even if I wait 10 seconds for it to load it just seems that after a click nothing happens in the test. At the same time I've found out that neither wait_for_turbo_frame_id nor wait_for_element_present helpers work correctly, because they were silently letting me through although the frame was not loaded.

2. I'm getting very confusing results when there is more than one summarizable column. It looks like some charts are loaded with wrong data and some are not being loaded at at all (although the numbers and legend is being rendered correctly). Might be something related to the Chartkick gem, not sure. I'd need to debug it more. You can see this confusing behaviour in the video below.

https://github.com/avo-hq/avo/assets/12682792/434be96f-9999-41b7-b1e1-050add738871

I'd appreciate some help here @adrianthedev :)

[adrianthedev]

Hey @bryszard. For sure I'll help. I'm traveling for a bit right now, so it might take a while, but I'll definitely dig in and get my hands dirty. Thank you for working on this!

[bryszard]

@adrianthedev Found the solution to the problem 2 - we just need to add explicit ids to the charts (auto-increment on the default ids don't work correctly).

The only problem left is the Capybara :).

[bryszard]

Ok, got to the source of the problem with Capybara too. I was assuming that :feature tests are running JavaScript, but it turned out that they use the default RackTest driver which doesn't trigger JS.

I've changed the type of test to :system (using cuprite) and it works :).

[bryszard]

There are some flaky system tests in spec/system/avo/date_time_fields/time_spec.rb. Unfortunately, I cannot reproduce the problem locally. And it doesn't seem to be related to my changes.

[adrianthedev]

Hey @bryszard. Amazing work! Thanks for taking this through it's paces! Really deep and thoughtful! Thank you!

I pushed a few tweaks to make the design look a bit more in line with what we have and same for a couple of code patterns. I also added a blank layout to make the distribution frames load up faster without thinking about the header, sidebar, and other parts of the app. It's like 10-20-40 times faster. I'll add this to cards loading as well.

There's one more thing this feature needs, but not necessarily right now. When you view the table in an association, the distribution is queried outside of the scope of the association. So if you have 4 projects for a user, when you click a distribution link it will show the graph for all projects in the database. Please let me know if you'd like to tackle that now, or you prefer at a later time when someone complains about it. If we don't do it now, we just need to disable the distribution icon when the table header has a @parent_record.

Thank you!

[bryszard]

Hey @adrianthedev, thank you for the thorough review.

I'd prefer to do it iteratively, so I'll just disable it for associations for now. Nice that you cought it!

[bryszard]

Thanks @adrianthedev for doing this last step!

[adrianthedev]

Yes, sure! I was on the branch and it was easy to toggle it.

Thanks for all your work!

[xeron]

Just tried this feature and got a error:

undefined method `pie_chart' for #<ActionView::Base:0x0000000002fd78>

Seems like chartkick in not installed even after doing bundle install or bundle update.

[adrianthedev]

Try to add chartkick to your Gemfile. We need to document this fact.

[xeron]

Yes this works. Thanks! For some reason I expected it to be installed as an avo dependency.

[adrianthedev]

It's not a dependency because not everyone uses it from the start, so we don't want pile up another dependency ✌️