OCA
[16.0][IMP] - css sticky table header added on mis-builder reports
Thanks for your pull request. Please take a moment to review if it meets the following guidelines.
Description
Following the discussion on the PR for the same feature on V14 https://github.com/OCA/mis-builder/pull/588. The PR adds a sticky header and first column on the Mis budget report. On the first gif you can see the vertical scroll, on the second one the horizontal one.
Thx !
Hi @sbidoul, some modules you are maintaining are being modified, check this out!
Hello,
I tested (not on runboat) on a copy of a database we have with a wide report already configured. Unfortunately, the display is not so good, as you can see:
For comparison, this is the report with non sticky header in production database:
The stickyness is working for the title line, but the whole rendering is not working. Is it something in my database?!
From a technical standpoint, I worry that these overrides of standard odoo css classes would have side effects in other contexts than mis builder? Not sure.
Also, is there a better solution than hard coding colors?
Thank you all for the feedback. Ia I was advised, I decided to split in 2 different PRs the two features : sticky header + sticky first column. I added a commit with a refactor keeping only the sticky header in this PR. I hope it would be easier to test.
@sbidoul , thank you, I changed the colors by using the standard odoo bootstrap variables.
Also, to reinsure that the classes don't interfere or override classes on other tables / views, I further nested the css classes with those only used on miss builder , for example :
.oe_mis_builder_content .o_list_renderer thead th { position: -webkit-sticky; position: sticky; top: 0; background-color: var(--gray-300); z-index: 1; border: 0.2px solid var(--gray-100); }
Here the style applies only on html table header elements where the parent has oe_mis_builder_content class (and o_list_renderer ) applied . This ensures that the style won't apply on other odoo tables or lists. @vdewulf , would be great to see if my refactor works on your reports ! Thank you for you collaboration and testing !
@dora-jurcevic I tested the last version on the runboat here and it doesn't seem to work.
See my video.
@dora-jurcevic I tested the last version on the runboat here and it doesn't seem to work. See my video.
Thank you @vdewulf for testing. So on the OCA bot the testing is not really ideal beacuse the table has only 3 rows. For the sticky header to appear, the table should be bigger (heigher) than the screen . You can "cheat" by :
- inspecting the table
- selecting a
element in the inspector tool - right click -> duplicate node
- this will copy a row , and if you do it 15-20 times (easy peasy ;) ) , you can simulate a big table without having to reproduce enough data for a big report.
Hope this helps !
I added many kpi's row in the runboat http://oca-mis-builder-16-0-pr673-458d6234f03c.runboat.odoo-community.org/web#id=1&cids=1&menu_id=120&action=281&model=mis.report&view_type=form the sticky header is working.
But i found that the report is squeezed in one page, this give difficult views if there are many columns in one report. It be good if the report can be freezed in the first column as well. thanks
@dora-jurcevic Test successfull on my customer database.
As stated together previously, as a next improvement, the first column should be fixed, but with an attention to fix the width of the column because some of our customers have really (really) long names of accounts, as you will see in this GIF.
You need to fix the pre-commit aspects (no idea how) before we can merge this, after answering the last comments from @sbidoul. Thanks
@dora-jurcevic I made https://github.com/akretion/mis-builder/pull/1 with the pre-commit fix and the changes I suggested above.
I made akretion#1 with the pre-commit fix and the changes I suggested above.
@dora-jurcevic can you merge akretion#1 if ok for you?
This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖
I can do it in anoter PR if you prefer, preserving your authorship (of course).
hello @sbidoul ! Please see this MR, I rebased the code and everything seems to be green now. Thx !
On my way to merge this fine PR! Prepared branch 16.0-ocabot-merge-pr-673-by-sbidoul-bump-minor, awaiting test results.
Congratulations, your PR was merged at 39da6eeebd3cf2cd7f1b1c9266514a57a2ddbe32. Thanks a lot for contributing to OCA. ❤️