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

UI updates/ Launch Lessons page #12647

Merged
merged 12 commits into from
Dec 13, 2024
Prev Previous commit
Next Next commit
refactor Unit component, add onClick to DataTableChip
  • Loading branch information
eadams17 committed Dec 9, 2024
commit b736c48fbbf3afdd390b1595576a674af0d55aeb
11 changes: 11 additions & 0 deletions services/QuillLMS/app/assets/stylesheets/pages/my_lessons.scss
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@
}
}
}
.lesson-activities-table {
.data-table-chip {
max-width: 300px;
width: max-content;
p {
text-overflow: unset;
white-space: break-spaces;
text-align: left;
}
}
}
}
.empty-lessons-content {
display: flex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ interface DataTableChipProps {
src: string
},
label: string,
link?: string
link?: string,
onClick?: () => void
}

export const DataTableChip = ({ color, icon, label, link }: DataTableChipProps) => {
export const DataTableChip = ({ color, icon, label, link, onClick }: DataTableChipProps) => {
if(link) {
return(
<a className={`data-table-chip ${color} focus-on-light`} href={link}>
Expand All @@ -19,6 +20,14 @@ export const DataTableChip = ({ color, icon, label, link }: DataTableChipProps)
</a>
)
}
if (onClick) {
return (
<button className={`data-table-chip ${color} focus-on-light`} onClick={onClick}>
Copy link
Member

Choose a reason for hiding this comment

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

Probably need a button type declaration here for the linter, right?

{icon && <img alt={icon.alt} src={icon.src} />}
<p>{label}</p>
Copy link
Member

Choose a reason for hiding this comment

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

I think a span is the more appropriate semantic element here -- I've been getting a lot of console errors about nesting block elements inside inline elements, which I think is technically not allowed under the HTML5 specification. There are some cases where it's tricky to avoid (like nesting elements in tables), but I think we should try where we can.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok! I'll make that change. Thank you for the context

</button>
)
}
return (
<div className={`data-table-chip ${color}`}>
{icon && <img alt={icon.alt} src={icon.src} />}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ export default class ClassroomLessons extends React.Component {
}

render() {
console.log('classroom lessons component')
const { empty, loaded, classrooms, lessons, selectedClassroomId, } = this.state
if (empty) {
return this.renderEmptyState();
Expand Down
Loading