adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.95k stars 1.12k forks source link

alignment is wrong when isHidden is set to true for Text inside ActionButton with icon #4946

Open prabirshrestha opened 1 year ago

prabirshrestha commented 1 year ago

Provide a general summary of the issue here

For responsive design I would like to remove <Text> when in mobile view but show it when in larger screens.

๐Ÿค” Expected Behavior?

Correct alignment.

image

๐Ÿ˜ฏ Current Behavior

left padding is bigger than the right instead of center aligned.

image

๐Ÿ’ Possible Solution

isHidden should be respected for now I'm using the following workaround.

export const useInBaseReponsiveMode = () => {
  const { matchedBreakpoints } = useBreakpoint();
  const isBase = useMemo(() => {
    return getResponsiveProp(
      {
        base: true,
        M: false,
      },
      matchedBreakpoints
    );
  }, [matchedBreakpoints]);
  return isBase;
};

return (
  <ActionButton>
    <Add />
    {!isBase && <Text>New</Text>}
  </ActionButton>
);

Seems like a hack.

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

Set isHidden for ActionButton and resize the window.

<ActionButton>
  <Add />
  <Text isHidden={{ base: true, M: false }}>New</Text>
</ActionButton>

Version

3.29.0

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

linux

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

snowystinger commented 1 year ago

This is controlled by the css, which currently has no concept of hidden/visible text; it only knows if there is a node. https://github.com/adobe/react-spectrum/blob/8987f9283871b1bda0acc4637096966275f82167/packages/%40adobe/spectrum-css-temp/components/button/index.css#L147

We might be able to add some :not([hidden]).

Your "workaround" is how I would recommend solving the issue, though, without that extra css. I don't really feel like it's a hack; that is what the hook is there for, to solve odd cases like this. We have only removed/added the label in cases where the container is a certain size compared to the button, so we've never run across this use case.

You could submit a PR adding the appropriate CSS selectors to the existing rules if you'd like.