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.

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

Cleanup parameters popover #42107

Merged
merged 7 commits into from May 2, 2024
Merged

Cleanup parameters popover #42107

merged 7 commits into from May 2, 2024

Conversation

uladzimirdev
Copy link
Contributor

@uladzimirdev uladzimirdev commented May 1, 2024

closes #42163

Drop not used components and rework file to TS.

no-backport as it depends on other changes which weren't backported

@uladzimirdev uladzimirdev changed the title cleanup parameters popover Cleanup parameters popover May 1, 2024
Copy link

replay-io bot commented May 1, 2024

Status In Progress ↗︎ 51 / 52
Commit 9014dd8
Results
⚠️ 8 Flaky
2446 Passed

@uladzimirdev uladzimirdev added the no-backport Do not backport this PR to any branch label May 2, 2024
@uladzimirdev uladzimirdev requested review from a team May 2, 2024 15:00
Copy link
Contributor

@heypoom heypoom left a comment

Choose a reason for hiding this comment

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

Looks good!

PS. Might also be good to use Mantine components in some of these, but we can do that in a later PR

@@ -89,28 +78,3 @@ export const ParameterOptionsSectionsPane = ({ sections, onSelectSection }) => (
</ul>
</PopoverBody>
);

export const ParameterOptionItem = ({ option, onClick }) => (
<OptionItemRoot onClick={onClick}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not used

name={getParameterIconName(section.id)}
className={CS.mr1}
/>
<Icon size="16" name="label" className={CS.mr1} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

typescript showed that getParameterIconName expected parameter object and not id, so it never worked as intended and nobody noticed. I just hardcoded the fallback value from getParameterIconName

@uladzimirdev uladzimirdev enabled auto-merge (squash) May 2, 2024 20:43
@uladzimirdev uladzimirdev merged commit c3d2378 into master May 2, 2024
112 checks passed
@uladzimirdev uladzimirdev deleted the cleanup-parameters-popover branch May 2, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport Do not backport this PR to any branch .Team/QueryingComponents
Projects
None yet
2 participants