Skip to content

Commit

Permalink
fix(alert): no wrapper class when no message provided (#389)
Browse files Browse the repository at this point in the history
Currently when there's no message but has children (explanations), then
a `m-notification_explanation` applied which makes it not possible to
achieve designs like these (unbolded, aligned text):

<img width="1143" alt="Screenshot 2024-11-19 at 4 13 26 PM"
src="https://github.com/user-attachments/assets/e78ff4a4-6c08-4df8-9455-4e6648a57701">

<img width="648" alt="Screenshot 2024-11-19 at 4 16 46 PM"
src="https://github.com/user-attachments/assets/6f053344-afef-4b12-a8fe-4efcf264cc8c">


## Changes

- if there is no message, but still has children then do not apply
`m-notification_explanation` to allow alert flexibility

## How to test this PR

1. Do the tests pass?
2. How do the examples look?
3. Try modifying the package.json to point the DSR to this commit and
see how the alerts are affected.

## Screenshots
<img width="1346" alt="Screenshot 2024-11-19 at 4 21 12 PM"
src="https://github.com/user-attachments/assets/add6b874-260f-4e77-b6a8-6ddbeb5ecfbb">
  • Loading branch information
billhimmelsbach authored Nov 20, 2024
1 parent 874fdba commit 87e95f8
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 4 deletions.
15 changes: 12 additions & 3 deletions src/components/Alert/Alert.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,21 @@ export const Information: Story = {
args: { status: 'info', message: 'A Notification' }
};

export const InformationWithExplanation: Story = {
export const InformationWithMessageAndExplanation: Story = {
...Information,
name: 'Information with explanation',
name: 'Information with a message and an explanation',
args: {
...Information.args,
children: 'You can also add an explanation to the notification.'
message: 'Here is the message of the notification.',
children: 'Here is the explanation of the notification.'
}
};

export const InformationWithOnlyExplanation: Story = {
...Information,
name: 'Information with only an explanation',
args: {
children: 'You can also only have an explanation in the notification.'
}
};

Expand Down
12 changes: 12 additions & 0 deletions src/components/Alert/Alert.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import '@testing-library/jest-dom';
import { render, screen, within } from '@testing-library/react';
import Paragraph from '../Paragraph/Paragraph';
import { Alert, AlertType } from './Alert';
import { AlertFieldLevel } from './AlertFieldLevel';

Expand Down Expand Up @@ -45,6 +46,17 @@ describe('<Alert />', () => {
expect(explanation).toBeInTheDocument();
});

it('does not include an explanation wrapper class when there is no message but children are provided', async () => {
render(
<Alert status='info'>
<Paragraph>Test component</Paragraph>
</Alert>
);
const explanation = screen.queryByTestId('explanation');
expect(explanation).toBeInTheDocument();
expect(explanation).not.toHaveClass('m-notification_explanation');
});

it('displays links when provided', async () => {
render(<Alert status='info' />);
const noLinks = screen.queryAllByRole('listitem');
Expand Down
5 changes: 4 additions & 1 deletion src/components/Alert/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ export const Alert = ({
</p>
) : null}
{children ? (
<div className='m-notification_explanation' data-testid='explanation'>
<div
className={`${message ? 'm-notification_explanation' : ''}`}
data-testid='explanation'
>
{children}
</div>
) : null}
Expand Down

0 comments on commit 87e95f8

Please sign in to comment.