fix: reposition column picker to always be visible

[roberbnd]

What Add value to property "left" if we haven't space available in the viewport default is 10 to maintain the same behavior if it is not necessary

Why when you click on the edge of the grid the columns options is not totally visible

[stackblitz[bot]]

Run & review this pull request in StackBlitz Codeflow.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.8%. Comparing base (21900b9) to head (6339f43). Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
...ackages/common/src/extensions/slickColumnPicker.ts	66.7%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1704 +/- ## ======================================== - Coverage 99.8% 99.8% -0.0% ======================================== Files 187 187 Lines 31093 31098 +5 Branches 9757 9758 +1 ======================================== + Hits 31004 31007 +3 - Misses 89 91 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[ghiscoding]

@roberbnd please provide more details and necessary unit test (I can help if necessary). I would like more info on where you took the number 60 in your calculation or is that a random number? We shouldn't have fixed numbers in code, it's hard to find and debug (it could be moved to a constant on top of the file or as a new grid menu option). So if you could please provide more details, print screen or animated gif would also be useful to understand the problem. Thanks

[roberbnd]

@ghiscoding I want to avoid this: as you can see the table options "columns" is in the edge.

60 is a "random" number after I was testing how much I need to move the <div> to the left

about testing I didn't know it was necessary for this PR, I am working on that part but I dont get it what is the command only one file execute packages/common/src/extensions/tests/slickColumnPicker.spec.ts I already did pnpm install

[ghiscoding]

Ahh ok I thought this was for the Grid Menu (which already has an auto-positioning) but I just realized that it's for the Column Picker which doesn't have any auto-positioning. However your code seems more like a patch for your use case, However, I think what could be better is to actually add an auto-positioning the same way that I've done for the Header Menu (which has a left/right position depending on which side has more space available), so I think we should go for that approach instead. If you take a look at the Header Menu, here's the code that does this left/right positioning

https://github.com/ghiscoding/slickgrid-universal/blob/6491d51ad146b7e6028fe020686cda76f455a000/packages/common/src/extensions/slickHeaderMenu.ts#L168-L195

We could copy this code and do something similar, but since I also like to stay DRY (don't repeat yourself), I think we should move this into a separate function that could be called by both (probably in extensionCommonUtils.ts). However this might feel like a lot of work for you, so I could take it over if you think that is too much work... or you could still contribute, it's really up to you. So in summary, I would rather go with an auto-positioning option similar to the Header Menu plugin (as shown in the animated gif below)

and here's how to add breakpoint unit tests, it probably only works if you use VSCode though. The reason I asked for unit test is because I worked really, really hard to get nearly 100% test coverage (5K tests) and I certainly don't want it to decrease, it's hard but it helps a lot to catch bugs and roll new versions (especially when rolling a major version) 😉

[ghiscoding]

@roberbnd I got it working as above with just couple lines, if you want to contribute, you could apply these changes (slickColumnPicker.ts)

repositionMenu() { 
  // auto-positioned menu left/right by available position
+ const gridPos = this.grid.getGridPosition();
+ const menuWidth = this._menuElm.clientWidth || 0;
+ let menuOffsetLeft = targetEvent.pageX || 0;
+ if (gridPos?.width && (menuOffsetLeft + menuWidth >= gridPos.width)) {
+   menuOffsetLeft = menuOffsetLeft - menuWidth;
+ }

  this._menuElm.style.top = `${targetEvent.pageY - 10}px`;
- this._menuElm.style.left = `${targetEvent.pageX - 10}px`;
+ this._menuElm.style.left = `${menuOffsetLeft}px`;
  // ...
}

and then add this unit test to cover the change (slickColumnPicker.spec.ts)

const gridStub = {
  applyHtmlCode: (elm, val) => elm.innerHTML = val || '',
+  getGridPosition: vi.fn(),
  getColumnIndex: vi.fn(),

// go to line 365, add unit test just before "onColumnsReordered event"

+    it('should reposition menu to the left when no available space on the right', () => {
+      vi.spyOn(gridStub, 'getGridPosition').mockReturnValue({ left: 50, top: 0, right: 0, bottom: 200, height: 22, width: 300, visible: true });
+      vi.spyOn(gridStub, 'getColumnIndex').mockReturnValue(undefined as any).mockReturnValue(1);
+
+      control.init();
+
+      const groupElm = createDomElement('div', { className: 'slick-column-name' });
+      gridStub.onPreHeaderContextMenu.notify({ node: groupElm, grid: gridStub }, { ...new SlickEventData(), preventDefault: vi.fn(), target: groupElm, pageX: 305 } as any, gridStub);
+      vi.spyOn(control, 'createPickerMenu').mockImplementation(() => {
+        if (control.menuElement) {
+          Object.defineProperty(control.menuElement, 'clientWidth', { writable: true, value: 122 });
+          return control.menuElement;
+        }
+        return document.createElement('div');
+      });
+      gridStub.onPreHeaderContextMenu.notify({ node: groupElm, grid: gridStub }, { ...new SlickEventData(), preventDefault: vi.fn(), target: groupElm, pageX: 305 } as any, gridStub);
+      Object.defineProperty(control.menuElement, 'clientWidth', { writable: true, value: 122 });
+      expect(control.menuElement?.style.left).toBe('183px');
+    });

    describe('onColumnsReordered event', () => {
      it('should reorder some columns', () => {
}

[ghiscoding]

Since I didn't hear anything back, I went ahead and modified your PR and superseded it as a new PR #1707 Also since I checked out your PR, your commit will be part of the git history and you will still be added as a contributor.

Thanks for opening this.