kgabryje commented on code in PR #27701:
URL: https://github.com/apache/superset/pull/27701#discussion_r1551671417
##########
superset-frontend/packages/superset-ui-core/src/hooks/useTruncation/useChildElementTruncation.ts:
##########
@@ -27,92 +27,68 @@ import { RefObject, useLayoutEffect, useState, useRef }
from 'react';
* (including those completely hidden) and whether any elements
* are completely hidden.
*/
-const useChildElementTruncation = (
- elementRef: RefObject<HTMLElement>,
- plusRef?: RefObject<HTMLElement>,
-) => {
+const useChildElementTruncation = () => {
const [elementsTruncated, setElementsTruncated] = useState(0);
const [hasHiddenElements, setHasHiddenElements] = useState(false);
-
- const previousEffectInfoRef = useRef({
- scrollWidth: 0,
- parentElementWidth: 0,
- plusRefWidth: 0,
- });
+ const elementRef = useRef<HTMLDivElement>(null);
+ const plusRef = useRef<HTMLDivElement>(null);
useLayoutEffect(() => {
- const currentElement = elementRef.current;
- const plusRefElement = plusRef?.current;
-
- if (!currentElement) {
- return;
- }
-
- const { scrollWidth, clientWidth, childNodes } = currentElement;
-
- // By using the result of this effect to truncate content
- // we're effectively changing it's size.
- // That will trigger another pass at this effect.
- // Depending on the content elements width, that second rerender could
- // yield a different truncate count, thus potentially leading to a
- // rendering loop.
- // There's only a need to recompute if the parent width or the width of
- // the child nodes changes.
- const previousEffectInfo = previousEffectInfoRef.current;
- const parentElementWidth = currentElement.parentElement?.clientWidth || 0;
- const plusRefWidth = plusRefElement?.offsetWidth || 0;
- previousEffectInfoRef.current = {
- scrollWidth,
- parentElementWidth,
- plusRefWidth,
- };
-
- if (
- previousEffectInfo.parentElementWidth === parentElementWidth &&
- previousEffectInfo.scrollWidth === scrollWidth &&
- previousEffectInfo.plusRefWidth === plusRefWidth
- ) {
- return;
- }
+ const onResize = () => {
+ const currentElement = elementRef.current;
+ if (!currentElement) {
+ return;
+ }
+ const plusRefElement = plusRef.current;
+ const { scrollWidth, clientWidth, childNodes } = currentElement;
- if (scrollWidth > clientWidth) {
- // "..." is around 6px wide
- const truncationWidth = 6;
- const plusSize = plusRefElement?.offsetWidth || 0;
- const maxWidth = clientWidth - truncationWidth;
- const elementsCount = childNodes.length;
+ if (scrollWidth > clientWidth) {
+ // "..." is around 6px wide
+ const truncationWidth = 6;
Review Comment:
Not really - the ellipsis is not something that we can grab, because it's
not an element in the DOM. What we could do to get a more precise measurement
is is render `<span>...</span>` somewhere off screen, measure the width, remove
that element and use that width here.
To be honest I'm not sure if it's worth it, the worst thing that can happen
here is an off-by-one error in some edge cases
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
