Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a series of virtual scrolling components along with supporting composables and updated styles for scrollbars and fade-out effects. Key changes include:
- New virtual components: FSVirtualWrap, FSVirtualRow, and FSVirtualCol.
- Updates to global scrollbar styles and FSFadeOut component styling.
- Introduction of the useElementPosition composable with integration in the virtual components.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/foundation-shared-components/styles/globals/scrollbars.scss | Adds custom scrollbar styling for horizontal and vertical scroll areas. |
| src/shared/foundation-shared-components/styles/components/*.scss | Introduces new SCSS files for virtual components layouts. |
| src/shared/foundation-shared-components/composables/* | Adds useScroll and useElementPosition composables for scroll and element measurement. |
| src/shared/foundation-shared-components/components/virtuals/*.vue | Implements new virtual scrolling components with layout and rendering logic. |
| dev/storybook/src/stories/shared/virtuals/* | Provides Storybook stories for visual testing and demonstration of the new virtual components. |
| <div | ||
| v-for="item in renderedItems" | ||
| :key="item.key" | ||
| :style="{ |
There was a problem hiding this comment.
C'est pas très joli ça non?
| x: currentX, | ||
| y: 0, | ||
| width: item.width, | ||
| height: '100%', |
There was a problem hiding this comment.
À quoi sert cette height (même question pour la hauteur dans la Col)
| @@ -0,0 +1,171 @@ | |||
| <template> | |||
| <FSRow | |||
There was a problem hiding this comment.
Je comprends pas trop l'intéret de cette FSRow, si c'était une col ou une div classique l'effet serait le même non? D'ailleurs en as-t-on vraiment besoin? Si on avait directement le FSFadeOut ça marcherait aussi bien?
There was a problem hiding this comment.
D'ailleurs je trouve pas que le nom du composant soit génial, ça ressemble plus à ce qu'on appelle un FadeOut qu'une col ou une row. Peut être qu'on pourrait l'appeler FSVirtualHorizontalFadeOut ou bien avoir juste un FSVirtualFadeOut qui gère les 3 cas.
| ref="root" | ||
| class="fs-virtual-wrap" | ||
| :height="$props.height" | ||
| v-bind="$attrs" |
There was a problem hiding this comment.
Je crois que v-bind="$attrs" est automatique au premier enfant
| } | ||
| i += maxItemsPerRow.value - 1; // on saute les items déjà traités | ||
| currentX = 0; | ||
| currentY += props.itemHeight + gap; |
There was a problem hiding this comment.
On pourrait pas faire comme les autres et définir la hauteur et la width dans les items? Histoire d'avoir une ligne de la hauteur qui correspond à la height de l'item le plus haut de la row?
| * @param callback fonction à appeler à chaque scroll ou intersection | ||
| * @param selectors liste de sélecteurs CSS pour trouver les parents scrollables (optionnel) | ||
| */ | ||
| export function useScroll(rootRef: any, callback: () => void, selectors: string[] = [".fs-fade-out"]) { |
There was a problem hiding this comment.
Il est utilisé où ce composable?
| const element = root.value.$el ? root.value.$el : root.value; | ||
| // Recherche des parents scrollables via selectors | ||
| parents = []; | ||
| for (const selector of selectors) { |
There was a problem hiding this comment.
Se serait cool si on pouvait faire ça à la main sans demander de fournir des sélecteurs. Peut etre avec ça (conseil de copilot) :
/**
* Retourne la liste des parents scrollables d'un élément DOM.
*/
export function getScrollableParents(element: HTMLElement): HTMLElement[] {
const parents: HTMLElement[] = [];
let node = element.parentElement;
while (node && node !== document.body) {
const style = getComputedStyle(node);
if (
/(auto|scroll)/.test(style.overflowY) && node.scrollHeight > node.clientHeight ||
/(auto|scroll)/.test(style.overflowX) && node.scrollWidth > node.clientWidth
) {
parents.push(node);
}
node = node.parentElement;
}
// Optionnel: inclure document.scrollingElement (le <html>), utile sur certaines pages
if (document.scrollingElement) {
parents.push(document.scrollingElement as HTMLElement);
}
return parents;
}| for (let index = 0; index < props.items.length; index++) { | ||
| const item = props.items[index]; | ||
| result.push({ | ||
| key: props.itemKey in item ? item[props.itemKey] as string : index, |
There was a problem hiding this comment.
Pourquoi on ne prend pas toujours l'index de l'item?
…d-ui into features/virtuals
…ty and style adjustments
No description provided.