mirror of
https://github.com/jlengrand/Maestro.git
synced 2026-03-10 08:31:19 +00:00
fix(tab-bar): ensure full tab visibility when scrolling into view
Changed from centering the tab to using scrollIntoView with 'nearest' option. This ensures the entire tab including the close button is visible, rather than potentially cutting off the right edge when near container boundaries.
This commit is contained in:
@@ -164,8 +164,9 @@ describe('TabBar', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.clearAllMocks();
|
||||
// Mock scrollTo
|
||||
// Mock scrollTo and scrollIntoView
|
||||
Element.prototype.scrollTo = vi.fn();
|
||||
Element.prototype.scrollIntoView = vi.fn();
|
||||
// Mock clipboard
|
||||
Object.assign(navigator, {
|
||||
clipboard: {
|
||||
@@ -1430,13 +1431,13 @@ describe('TabBar', () => {
|
||||
});
|
||||
|
||||
describe('scroll behavior', () => {
|
||||
it('scrolls to center active tab when activeTabId changes', async () => {
|
||||
it('scrolls active tab into view when activeTabId changes', async () => {
|
||||
// Mock requestAnimationFrame
|
||||
const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => {
|
||||
cb(0);
|
||||
return 0;
|
||||
});
|
||||
const scrollToSpy = vi.fn();
|
||||
const scrollIntoViewSpy = vi.fn();
|
||||
|
||||
const tabs = [
|
||||
createTab({ id: 'tab-1', name: 'Tab 1' }),
|
||||
@@ -1454,9 +1455,11 @@ describe('TabBar', () => {
|
||||
/>
|
||||
);
|
||||
|
||||
// Mock scrollTo on the container
|
||||
const tabBarContainer = container.firstChild as HTMLElement;
|
||||
tabBarContainer.scrollTo = scrollToSpy;
|
||||
// Mock scrollIntoView on the tab elements
|
||||
const tabElements = container.querySelectorAll('[data-tab-id]');
|
||||
tabElements.forEach((el) => {
|
||||
(el as HTMLElement).scrollIntoView = scrollIntoViewSpy;
|
||||
});
|
||||
|
||||
// Change active tab
|
||||
rerender(
|
||||
@@ -1470,19 +1473,29 @@ describe('TabBar', () => {
|
||||
/>
|
||||
);
|
||||
|
||||
// scrollTo should have been called via requestAnimationFrame
|
||||
expect(scrollToSpy).toHaveBeenCalled();
|
||||
// Re-mock scrollIntoView on tab elements after rerender
|
||||
const newTabElements = container.querySelectorAll('[data-tab-id]');
|
||||
newTabElements.forEach((el) => {
|
||||
(el as HTMLElement).scrollIntoView = scrollIntoViewSpy;
|
||||
});
|
||||
|
||||
// scrollIntoView should have been called via requestAnimationFrame
|
||||
expect(scrollIntoViewSpy).toHaveBeenCalledWith({
|
||||
inline: 'nearest',
|
||||
behavior: 'smooth',
|
||||
block: 'nearest',
|
||||
});
|
||||
|
||||
rafSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('scrolls to center active tab when showUnreadOnly filter is toggled off', async () => {
|
||||
it('scrolls active tab into view when showUnreadOnly filter is toggled off', async () => {
|
||||
// Mock requestAnimationFrame
|
||||
const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => {
|
||||
cb(0);
|
||||
return 0;
|
||||
});
|
||||
const scrollToSpy = vi.fn();
|
||||
const scrollIntoViewSpy = vi.fn();
|
||||
|
||||
const tabs = [
|
||||
createTab({ id: 'tab-1', name: 'Tab 1' }),
|
||||
@@ -1502,12 +1515,14 @@ describe('TabBar', () => {
|
||||
/>
|
||||
);
|
||||
|
||||
// Mock scrollTo on the container
|
||||
const tabBarContainer = container.firstChild as HTMLElement;
|
||||
tabBarContainer.scrollTo = scrollToSpy;
|
||||
// Mock scrollIntoView on the tab elements
|
||||
const tabElements = container.querySelectorAll('[data-tab-id]');
|
||||
tabElements.forEach((el) => {
|
||||
(el as HTMLElement).scrollIntoView = scrollIntoViewSpy;
|
||||
});
|
||||
|
||||
// Clear initial calls
|
||||
scrollToSpy.mockClear();
|
||||
scrollIntoViewSpy.mockClear();
|
||||
|
||||
// Toggle filter off - this should trigger scroll to active tab
|
||||
rerender(
|
||||
@@ -1522,19 +1537,29 @@ describe('TabBar', () => {
|
||||
/>
|
||||
);
|
||||
|
||||
// scrollTo should have been called when filter was toggled
|
||||
expect(scrollToSpy).toHaveBeenCalled();
|
||||
// Re-mock scrollIntoView on tab elements after rerender
|
||||
const newTabElements = container.querySelectorAll('[data-tab-id]');
|
||||
newTabElements.forEach((el) => {
|
||||
(el as HTMLElement).scrollIntoView = scrollIntoViewSpy;
|
||||
});
|
||||
|
||||
// scrollIntoView should have been called when filter was toggled
|
||||
expect(scrollIntoViewSpy).toHaveBeenCalledWith({
|
||||
inline: 'nearest',
|
||||
behavior: 'smooth',
|
||||
block: 'nearest',
|
||||
});
|
||||
|
||||
rafSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('scrolls to center file tab when activeFileTabId changes', async () => {
|
||||
it('scrolls file tab into view when activeFileTabId changes', async () => {
|
||||
// Mock requestAnimationFrame
|
||||
const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => {
|
||||
cb(0);
|
||||
return 0;
|
||||
});
|
||||
const scrollToSpy = vi.fn();
|
||||
const scrollIntoViewSpy = vi.fn();
|
||||
|
||||
const tabs = [createTab({ id: 'tab-1', name: 'Tab 1' })];
|
||||
const fileTab: FilePreviewTab = {
|
||||
@@ -1563,12 +1588,14 @@ describe('TabBar', () => {
|
||||
/>
|
||||
);
|
||||
|
||||
// Mock scrollTo on the container
|
||||
const tabBarContainer = container.firstChild as HTMLElement;
|
||||
tabBarContainer.scrollTo = scrollToSpy;
|
||||
// Mock scrollIntoView on the tab elements
|
||||
const tabElements = container.querySelectorAll('[data-tab-id]');
|
||||
tabElements.forEach((el) => {
|
||||
(el as HTMLElement).scrollIntoView = scrollIntoViewSpy;
|
||||
});
|
||||
|
||||
// Clear initial calls
|
||||
scrollToSpy.mockClear();
|
||||
scrollIntoViewSpy.mockClear();
|
||||
|
||||
// Select the file tab - this should trigger scroll to file tab
|
||||
rerender(
|
||||
@@ -1586,8 +1613,18 @@ describe('TabBar', () => {
|
||||
/>
|
||||
);
|
||||
|
||||
// scrollTo should have been called when file tab was selected
|
||||
expect(scrollToSpy).toHaveBeenCalled();
|
||||
// Re-mock scrollIntoView on tab elements after rerender
|
||||
const newTabElements = container.querySelectorAll('[data-tab-id]');
|
||||
newTabElements.forEach((el) => {
|
||||
(el as HTMLElement).scrollIntoView = scrollIntoViewSpy;
|
||||
});
|
||||
|
||||
// scrollIntoView should have been called when file tab was selected
|
||||
expect(scrollIntoViewSpy).toHaveBeenCalledWith({
|
||||
inline: 'nearest',
|
||||
behavior: 'smooth',
|
||||
block: 'nearest',
|
||||
});
|
||||
|
||||
rafSpy.mockRestore();
|
||||
});
|
||||
@@ -1878,6 +1915,7 @@ describe('TabBar', () => {
|
||||
querySelector: vi.fn().mockReturnValue({
|
||||
offsetLeft: 100,
|
||||
offsetWidth: 80,
|
||||
scrollIntoView: vi.fn(),
|
||||
}),
|
||||
scrollTo: vi.fn(),
|
||||
}),
|
||||
|
||||
@@ -1599,7 +1599,7 @@ function TabBarInner({
|
||||
const tabRefs = useRef<Map<string, HTMLDivElement>>(new Map());
|
||||
const [isOverflowing, setIsOverflowing] = useState(false);
|
||||
|
||||
// Center the active tab in the scrollable area when activeTabId or activeFileTabId changes, or filter is toggled
|
||||
// Ensure the active tab is fully visible (including close button) when activeTabId or activeFileTabId changes, or filter is toggled
|
||||
useEffect(() => {
|
||||
requestAnimationFrame(() => {
|
||||
const container = tabBarRef.current;
|
||||
@@ -1609,10 +1609,10 @@ function TabBarInner({
|
||||
`[data-tab-id="${targetTabId}"]`
|
||||
) as HTMLElement | null;
|
||||
if (container && tabElement) {
|
||||
// Calculate scroll position to center the tab
|
||||
const scrollLeft =
|
||||
tabElement.offsetLeft - container.clientWidth / 2 + tabElement.offsetWidth / 2;
|
||||
container.scrollTo({ left: scrollLeft, behavior: 'smooth' });
|
||||
// Use scrollIntoView with 'nearest' to ensure the full tab is visible
|
||||
// This scrolls minimally - only if the tab is partially or fully out of view
|
||||
// The 'end' option ensures the right edge (with close button) is visible
|
||||
tabElement.scrollIntoView({ inline: 'nearest', behavior: 'smooth', block: 'nearest' });
|
||||
}
|
||||
});
|
||||
}, [activeTabId, activeFileTabId, showUnreadOnly]);
|
||||
|
||||
Reference in New Issue
Block a user