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

Add classNameOverride to MultiSelectOption #21

Merged
merged 9 commits into from
Sep 4, 2023

Conversation

kaydwithers
Copy link
Contributor

@kaydwithers kaydwithers commented Sep 1, 2023

Why

This PR adds classNameOverride to the MultiSelectOption.

In Murmur the icon was not displaying correctly due to the reset.scss file using box-sizing: border-box; adding box-sizing: content-box; to the icon resolves this issue in Murmur.

With classNameOverride added I can now add a Tailwind box-content class.

Fixes REP-597.

What

Before
image

After
image

@kaydwithers kaydwithers requested a review from a team as a code owner September 1, 2023 06:30
@changeset-bot
Copy link

changeset-bot bot commented Sep 1, 2023

🦋 Changeset detected

Latest commit: 5d803a6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@kaizen/select Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@kaydwithers kaydwithers changed the title Rep 597 incorrect icon size in murmur fix: FilterMultiSelect incorrect icon size in Murmur Sep 1, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

✨ Here is your branch preview! ✨

Last updated for commit 5d803a6: fix: remove html attributes type

@gyfchong
Copy link
Contributor

gyfchong commented Sep 1, 2023

Sorry @kaydwithers we shouldn't be adding styles specifically to fix Murmur, since we can't check if all the other products won't break. You'll have to resolve this on your side.

@kaydwithers
Copy link
Contributor Author

@gyfchong I've updated the PR so that I can add classNameOverride to the component that I need to add content-box to.

import { Icon } from "@kaizen/component-library"
import check from "@kaizen/component-library/icons/check.icon.svg"
import { Badge } from "@kaizen/draft-badge"
import { MultiSelectItem } from "../../../types"
import { useSelectionContext } from "../../provider"
import styles from "./MultiSelectOption.module.scss"

export interface MultiSelectOptionProps {
export interface MultiSelectOptionProps
extends OverrideClassName<HTMLAttributes<HTMLSpanElement>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

You've also added native attributes to the props API but it's not implemented in the component. Let's just remove that part because it's going to add complications to this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @dougmacknz , that's done now.

@dougmacknz
Copy link
Contributor

I'll get you to update the PR title as well plz 🙏

@kaydwithers kaydwithers changed the title fix: FilterMultiSelect incorrect icon size in Murmur fix: add classNameOverride to MultiSelectOption Sep 4, 2023
@dougmacknz dougmacknz merged commit 3f71968 into main Sep 4, 2023
21 checks passed
@dougmacknz dougmacknz deleted the rep-597-incorrect-icon-size-in-murmur branch September 4, 2023 02:12
@dougmacknz dougmacknz changed the title fix: add classNameOverride to MultiSelectOption Add classNameOverride to MultiSelectOption Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants