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

Wrap long labels w/o spaces in structure to new lines #693

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Wrap long labels w/o spaces in structure to new lines #693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03350f3
Wrap long labels w/o spaces in structure to new lines
Dananji Oct 29, 2024
35dbecd
Fixes from code review
Dananji Oct 30, 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
10 changes: 6 additions & 4 deletions src/components/StructuredNavigation/NavUtils/List.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,14 @@ describe('List component', () => {
test('displays canvas level structure item w/o mediafragment as a span', () => {
expect(screen.queryByTestId('listitem-section-span')).toBeInTheDocument();
expect(screen.getByTestId('listitem-section-span'))
.toHaveTextContent('1. Lunchroom Manners09:32');
.toHaveTextContent('1.Lunchroom Manners09:32');
});

test('displays structures with the correct ListItems', () => {
expect(screen.getByText('1. Part I (00:45)'));
expect(screen.getByText('Part I (00:45)'));
expect(screen.getByText('Part I (00:45)').parentElement.tagName).toEqual('A');
expect(screen.getByText('Introduction'));
expect(screen.getByText('Introduction')).toHaveClass('ramp--structured-nav__item-title');
});
});

Expand Down Expand Up @@ -135,7 +137,7 @@ describe('List component', () => {
render(<ListWithManifest />);
expect(screen.queryByTestId('listitem-section-button')).toBeInTheDocument();
expect(screen.getByTestId('listitem-section-button'))
.toHaveTextContent('1. Lunchroom Manners');
.toHaveTextContent('1.Lunchroom Manners09:32');
});

describe('with playlist manifest', () => {
Expand Down Expand Up @@ -172,7 +174,7 @@ describe('List component', () => {
expect(screen.queryAllByTestId('list')).toHaveLength(1);
expect(screen.queryAllByTestId('list-item').length).toEqual(1);
expect(screen.queryAllByTestId('list-item')[0])
.toHaveTextContent('1. Beginning Responsibility: Lunchroom Manners (09:32)');
.toHaveTextContent('1.Beginning Responsibility: Lunchroom Manners (09:32)');
// Has tracker UI element attached
expect(screen.queryAllByTestId('list-item')[0].children[0]).toHaveClass('tracker');
});
Expand Down
7 changes: 5 additions & 2 deletions src/components/StructuredNavigation/NavUtils/ListItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,15 @@ const ListItem = ({
<div className="tracker"></div>
{isClickable ? (
<>
{isEmpty && <LockedSVGIcon />}
<a role='link'
className='ramp--structured-nav__item-link'
href={homepage && homepage != '' ? homepage : id}
onClick={handleClick}>
{`${itemIndex}. `}{label} {duration.length > 0 ? ` (${duration})` : ''}
{isEmpty && <LockedSVGIcon />}
{`${itemIndex}.`}
<span className='structured-nav__item-label' aria-label={label}>
{label} {duration.length > 0 ? ` (${duration})` : ''}
</span>
</a>
</>
) : (
Expand Down
25 changes: 12 additions & 13 deletions src/components/StructuredNavigation/NavUtils/ListItem.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ describe('ListItem component', () => {

test('renders successfully', () => {
expect(screen.getByTestId('list-item'));
expect(screen.getByText('1. Track 1. I Krafting (00:01:14)'));
expect(screen.getByText('Track 1. I Krafting (00:01:14)'));
});
});

Expand Down Expand Up @@ -227,9 +227,10 @@ describe('ListItem component', () => {
});

test('creates an anchor element and title for an item', () => {
const anchorElement = screen.getByText('2. Rinsing Well (00:05)');
expect(anchorElement.tagName).toEqual('A');
expect(anchorElement).toHaveAttribute(
const anchorElement = screen.getByText('Rinsing Well (00:05)');
expect(anchorElement.tagName).toEqual('SPAN');
expect(anchorElement.parentElement.tagName).toEqual('A');
expect(anchorElement.parentElement).toHaveAttribute(
'href',
'https://example.com/manifest/lunchroom_manners/canvas/1#t=165,170'
);
Expand Down Expand Up @@ -301,7 +302,7 @@ describe('ListItem component', () => {
});
render(<ListItemWithManifest />);
expect(screen.queryAllByTestId('list-item').length).toEqual(1);
expect(screen.queryAllByTestId('list-item')[0]).toHaveTextContent('1. Beginning Responsibility: Lunchroom Manners (09:32)');
expect(screen.queryAllByTestId('list-item')[0]).toHaveTextContent('Beginning Responsibility: Lunchroom Manners (09:32)');
expect(screen.queryAllByTestId('list-item')[0].getAttribute('data-label')).toEqual('Beginning Responsibility: Lunchroom Manners');
expect(screen.queryAllByTestId('list-item')[0].getAttribute('data-summary')).toEqual('Mind your manners!');
});
Expand All @@ -321,10 +322,9 @@ describe('ListItem component', () => {
render(<ListItemWithManifest />);

expect(screen.queryAllByTestId('list-item').length).toEqual(1);
expect(screen.queryByText('1. Beginning Responsibility: Lunchroom Manners (09:32)')).toBeInTheDocument();
expect(screen
.queryByText('1. Beginning Responsibility: Lunchroom Manners (09:32)')
.getAttribute('href'))
expect(screen.queryByText('Beginning Responsibility: Lunchroom Manners (09:32)')).toBeInTheDocument();
const structItem = screen.getByText('Beginning Responsibility: Lunchroom Manners (09:32)');
expect(structItem.parentElement.getAttribute('href'))
.toEqual('https://example.com/playlists/1?position=1');
});

Expand All @@ -342,10 +342,9 @@ describe('ListItem component', () => {
render(<ListItemWithManifest />);
expect(screen.queryAllByTestId('list')).toHaveLength(1);
expect(screen.queryAllByTestId('list-item').length).toEqual(3);
expect(screen.queryByText('1. Part I (00:45)')).toBeInTheDocument();
expect(screen
.queryByText('1. Part I (00:45)')
.getAttribute('href'))
expect(screen.queryByText('Part I (00:45)')).toBeInTheDocument();
const structItem = screen.getByText('Part I (00:45)');
expect(structItem.parentElement.getAttribute('href'))
.toEqual('https://example.com/manifest/lunchroome_manners/canvas/1#t=0.0,45.321');
});
});
Expand Down
16 changes: 7 additions & 9 deletions src/components/StructuredNavigation/NavUtils/SectionHeading.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,24 +84,22 @@ const SectionHeading = ({
'ramp--structured-nav__section',
isActiveSection ? 'active' : ''
)}
role="listitem" data-testid="listitem-section"
role='listitem' data-testid='listitem-section'
ref={sectionRef} data-label={label}
data-mediafrag={itemId ?? ''}>
<div className="section-head-buttons">
<div className='ramp--structured-nav__section-head-buttons'>
<button
data-testid={itemId == undefined ? "listitem-section-span" : "listitem-section-button"}
data-testid={itemId == undefined ? 'listitem-section-span' : 'listitem-section-button'}
ref={sectionRef} onClick={handleClick}
className={cx(
'ramp--structured-nav__section-title',
!itemId && 'not-clickable'
)}>
<span className="ramp--structured-nav__title"
aria-label={label}
>
{isRoot ? '' : `${itemIndex}. `}
{label}
<span className='ramp--structured-nav__title' aria-label={label}>
{isRoot ? '' : `${itemIndex}.`}
Dananji marked this conversation as resolved.
Show resolved Hide resolved
<span className='ramp--structured-nav__section-label'>{label}</span>
{duration != '' &&
<span className="ramp--structured-nav__section-duration">
<span className='ramp--structured-nav__section-duration'>
{duration}
</span>}
</span>
Expand Down
8 changes: 4 additions & 4 deletions src/components/StructuredNavigation/NavUtils/SectionHeading.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('SectionHeading component', () => {
expect(screen.queryAllByTestId('listitem-section-button')).toHaveLength(0);
expect(screen.queryByTestId('section-collapse-icon')).toBeInTheDocument();
expect(screen.getByTestId('listitem-section-span'))
.toHaveTextContent('1. Lunchroom Manners09:32');
.toHaveTextContent('1.Lunchroom Manners09:32');
});

test('renders canvas without children items', () => {
Expand All @@ -48,7 +48,7 @@ describe('SectionHeading component', () => {
expect(screen.queryAllByTestId('listitem-section-button')).toHaveLength(0);
expect(screen.queryByTestId('section-collapse-icon')).not.toBeInTheDocument();
expect(screen.getByTestId('listitem-section-span'))
.toHaveTextContent('1. Lunchroom Manners09:32');
.toHaveTextContent('1.Lunchroom Manners09:32');
});

test('renders canvas with mediafragment as a button', () => {
Expand All @@ -67,7 +67,7 @@ describe('SectionHeading component', () => {
expect(screen.queryAllByTestId('listitem-section-span')).toHaveLength(0);
expect(screen.queryAllByTestId('listitem-section-button')).toHaveLength(1);
expect(screen.getByTestId('listitem-section-button'))
.toHaveTextContent('1. Lunchroom Manners09:32');
.toHaveTextContent('1.Lunchroom Manners09:32');
expect(screen.getByTestId('listitem-section')).toHaveAttribute('data-mediafrag');
expect(screen.getByTestId('listitem-section').getAttribute('data-mediafrag'))
.toEqual('https://example.com/manifest/canvas#t=0.0,572');
Expand All @@ -91,7 +91,7 @@ describe('SectionHeading component', () => {
expect(screen.queryAllByTestId('listitem-section-span')).toHaveLength(1);
expect(screen.queryAllByTestId('listitem-section-button')).toHaveLength(0);
expect(screen.getByTestId('listitem-section-span'))
.toHaveTextContent('1. Lunchroom Manners09:32');
.toHaveTextContent('1.Lunchroom Manners09:32');
expect(screen.queryByTestId('section-collapse-icon')).not.toBeInTheDocument();
expect(screen.getByTestId('listitem-section')).toHaveAttribute('data-mediafrag');
expect(screen.getByTestId('listitem-section').getAttribute('data-mediafrag')).toEqual('');
Expand Down
47 changes: 34 additions & 13 deletions src/components/StructuredNavigation/StructuredNavigation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
color: $primaryGreenDark;
transition: 0.25s;
text-decoration: none;
display: inline-flex;
gap: 0.1em;

&:hover {
color: $primaryDarker;
Expand All @@ -78,6 +80,10 @@
color: $primaryDarker;
}

span {
overflow-wrap: anywhere;
}

// Remove top border for first SectionHeading (causes double lines)
.ramp--structured-nav__section {
&:first-child {
Expand Down Expand Up @@ -106,6 +112,10 @@
ul.ramp--structured-nav__list>li:last-child {
padding: 0
}
// Change display to align locked-icon with label in playlists
a {
display: inline-block;
}
}

// Border box
Expand Down Expand Up @@ -175,6 +185,10 @@ ul.ramp--structured-nav__list {
// Set line-height to create clickable space around link
.ramp--structured-nav__item-link {
line-height: 1.65em;

.structured-nav__item-label {
margin-left: 0.2em;
}
}

ul {
Expand Down Expand Up @@ -219,7 +233,7 @@ ul.ramp--structured-nav__list {
}

.ramp--structured-nav__section {
.section-head-buttons {
.ramp--structured-nav__section-head-buttons {
display: grid;
grid-template-columns: 1fr auto;

Expand Down Expand Up @@ -281,18 +295,25 @@ ul.ramp--structured-nav__list {
}
}

// span {
// padding: 1rem;
// }
.ramp--structured-nav__section-title {
display: flex;
gap: 0.2em;

span.ramp--structured-nav__section-duration {
border: 1px solid $primaryDark;
border-radius: 999px;
color: $primaryDarkest;
font-size: 0.75rem;
letter-spacing: 0.02rem;
line-height: 1.6;
padding: 0 0.5rem;
margin-left: 0.5rem;
span.ramp--structured-nav__section-label {
overflow-wrap: anywhere;
margin-left: 0.2em;
}

span.ramp--structured-nav__section-duration {
border: 1px solid $primaryDark;
border-radius: 999px;
color: $primaryDarkest;
font-size: 0.75rem;
letter-spacing: 0.02rem;
line-height: 1.6;
padding: 0 0.5rem;
margin-left: 0.5rem;
text-wrap: nowrap;
}
}
}
17 changes: 10 additions & 7 deletions src/components/StructuredNavigation/StructuredNavigation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('StructuredNavigation component', () => {

const rootRange = screen.getAllByTestId('listitem-section')[0];
expect(rootRange).toHaveClass('ramp--structured-nav__section');
expect(rootRange.children[0]).toHaveClass('section-head-buttons');
expect(rootRange.children[0]).toHaveClass('ramp--structured-nav__section-head-buttons');
expect(rootRange.children[0].children[0])
.toHaveAttribute('data-testid', 'listitem-section-span');
expect(rootRange.children[0].children[0])
Expand All @@ -82,8 +82,8 @@ describe('StructuredNavigation component', () => {
expect(screen.queryByText('Table of Contents')).toBeInTheDocument();

const firstCanvas = screen.queryAllByTestId('listitem-section')[1];
expect(firstCanvas.children[0]).toHaveTextContent('1. Lunchroom Manners11:00');
expect(firstCanvas.children[0]).toHaveClass('section-head-buttons');
expect(firstCanvas.children[0]).toHaveTextContent('1.Lunchroom Manners11:00');
expect(firstCanvas.children[0]).toHaveClass('ramp--structured-nav__section-head-buttons');
expect(firstCanvas.children[0].children[1]).toHaveClass('collapse-expand-button');
});
});
Expand Down Expand Up @@ -112,14 +112,17 @@ describe('StructuredNavigation component', () => {
});

test('first item is a section title as a span', () => {
expect(screen.queryAllByTestId('listitem-section').length).toBeGreaterThan(0);
expect(screen.queryAllByTestId('listitem-section')[0])
.toHaveTextContent('1. CD1 - Mahler, Symphony No.309:32');
expect(screen.queryAllByTestId('listitem-section').length).toBe(2);
expect(screen.queryAllByTestId('listitem-section')[0].children[0])
.toHaveTextContent('1.CD1 - Mahler, Symphony No.309:32');

// Two sections w/o Canvas references are rendered as text
expect(screen.queryAllByTestId('listitem-section-span').length).toBe(2);

const firstItem = screen.getAllByTestId('list-item')[0];
expect(firstItem).toHaveAttribute('data-label', 'Track 1. I. Kraftig');
expect(firstItem).toHaveTextContent(
'1. Track 1. I. Kraftig (06:14)'
'1.Track 1. I. Kraftig (06:14)'
);
expect(firstItem).toHaveClass(
'ramp--structured-nav__list-item'
Expand Down