Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: 15752 invalid tooltip on hovering the select component #15788

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: 15752 invalid tooltip on hovering the select component #15788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
25ad531
fix: 15752 invalid tooltip on hovering the select component
mHuzefa Feb 20, 2024
18f643f
fix: type declarations error while building
mHuzefa Feb 20, 2024
926432a
fix: modifying SelectItem value of format option-2
mHuzefa Feb 27, 2024
0889cb5
fix: formatting the file
mHuzefa Feb 27, 2024
397ac89
fix: removed the imperative selector as it is not necessary
mHuzefa Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions packages/react/src/components/Select/Select.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ export const Default = () => {
value="An example option that is really long to show what should be done to handle long text"
text="An example option that is really long to show what should be done to handle long text"
/>
<SelectItem value="Option 2" text="Option 2" />
<SelectItem value="Option 3" text="Option 3" />
<SelectItem value="Option 4" text="Option 4" />
<SelectItem value="option-2" text="Option 2" />
<SelectItem value="option-3" text="Option 3" />
<SelectItem value="option-4" text="Option 4" />
</Select>
</div>
);
Expand All @@ -101,10 +101,10 @@ export const Inline = () => {
labelText="Select"
helperText="Optional helper text">
<SelectItem value="" text="" />
<SelectItem value="Option 1" text="Option 1" />
<SelectItem value="Option 2" text="Option 2" />
<SelectItem value="Option 3" text="Option 3" />
<SelectItem value="Option 4" text="Option 4" />
<SelectItem value="option-1" text="Option 1" />
<SelectItem value="option-2" text="Option 2" />
<SelectItem value="option-3" text="Option 3" />
<SelectItem value="option-4" text="Option 4" />
</Select>
</div>
);
Expand All @@ -124,7 +124,7 @@ export const _WithLayer = () => (
value="An example option that is really long to show what should be done to handle long text"
text="An example option that is really long to show what should be done to handle long text"
/>
<SelectItem value="Option 2" text="Option 2" />
<SelectItem value="option-2" text="Option 2" />
</Select>
)}
</WithLayer>
Expand All @@ -143,9 +143,9 @@ export const Playground = (args) => {
value="An example option that is really long to show what should be done to handle long text"
text="An example option that is really long to show what should be done to handle long text"
/>
<SelectItem value="Option 2" text="Option 2" />
<SelectItem value="Option 3" text="Option 3" />
<SelectItem value="Option 4" text="Option 4" />
<SelectItem value="option-2" text="Option 2" />
<SelectItem value="option-3" text="Option 3" />
<SelectItem value="option-4" text="Option 4" />
</Select>
</div>
);
Expand Down
13 changes: 12 additions & 1 deletion packages/react/src/components/Select/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import React, {
ForwardedRef,
ReactNode,
useContext,
useEffect,
useRef,
useState,
} from 'react';
Expand Down Expand Up @@ -157,6 +158,7 @@ const Select = React.forwardRef(function Select(
warnText,
onChange,
slug,
defaultValue,
...other
}: SelectProps,
ref: ForwardedRef<HTMLSelectElement>
Expand Down Expand Up @@ -227,7 +229,8 @@ const Select = React.forwardRef(function Select(
};

const handleChange = (evt) => {
setTitle(evt?.target?.value);
const selectedIndex = evt?.target?.selectedIndex;
setTitle(evt?.target?.options[selectedIndex]?.text);
};

const readOnlyEventHandlers = {
Expand Down Expand Up @@ -255,6 +258,13 @@ const Select = React.forwardRef(function Select(
size: 'mini',
});
}
useEffect(() => {
const selectElement = document.getElementById(
id
) as HTMLSelectElement | null;
setTitle(selectElement?.options[selectElement?.selectedIndex]?.text || '');
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Comment on lines +266 to +267
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to run on every render of the component? If so, I don't think it needs to be in a useEffect. If the select has a ref on it, can we use the ref to get the selected index/element instead of using document.getElementById?

We have a mergeRefs helper function that can be used to combine the forwarded ref and a local ref inside the component for this use if it's needed.


const input = (() => {
return (
Expand All @@ -263,6 +273,7 @@ const Select = React.forwardRef(function Select(
{...other}
{...ariaProps}
id={id}
defaultValue={defaultValue}
className={inputClasses}
disabled={disabled || undefined}
aria-invalid={invalid || undefined}
Expand Down
10 changes: 10 additions & 0 deletions packages/react/src/components/Select/__tests__/Select-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,16 @@ describe('Select', () => {
`${prefix}--select--slug`
);
});

it('should show SelectItem text as title', () => {
render(
<Select id="select" labelText="Select" defaultValue="option-2">
<SelectItem text="Option 1" value="option-1" />
<SelectItem text="Option 2" value="option-2" />
</Select>
);
expect(screen.getByLabelText('Select').title).toEqual('Option 2');
});
});

describe('behaves as expected', () => {
Expand Down
Loading