Conversation
| return ( | ||
| <span> | ||
| {parts.map((part, index) => { | ||
| if (part.startsWith("[") && part.endsWith("]")) { | ||
| const bracketsRemoved = part.slice(1, -1); | ||
| return ( | ||
| <span key={index} className={className}> | ||
| {bracketsRemoved} | ||
| </span> | ||
| ); | ||
| } | ||
| return part; | ||
| })} | ||
| </span> | ||
| ); |
There was a problem hiding this comment.
Some of these were breaking in undesired ways and a wrapping span seems to fix it.
| rows.push({ | ||
| key: "f_primer", | ||
| label: "F Primer for PCR/Sequencing:", | ||
| label: <>F Primer for PCR/<wbr />Sequencing:</>, |
There was a problem hiding this comment.
Very basic technique I have not used enough, tell it where to break the line.
There was a problem hiding this comment.
Pull request overview
This PR addresses mobile responsiveness issues in mutation and clone tables by fixing label wrapping and adjusting spacing for improved display on smaller screens.
Changes:
- Added explicit text wrapping for mutation table labels on mobile screens
- Adjusted padding for clone table cells to prevent overflow on mobile devices
- Refactored primer labels to use
<wbr>tags for controlled line breaking
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/style/info-panel.module.css | Added width constraint and text wrapping for description labels on mobile |
| src/style/clone-table.module.css | Added responsive padding for table headers and cells on mobile |
| src/components/SubPage/EditingDesign.tsx | Wrapped formatting function output in span element and added line break hints for primer labels |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .container :global(.ant-descriptions-item-label) { | ||
| font-size: 12px; | ||
| width: 88px; | ||
| text-wrap: wrap; |
There was a problem hiding this comment.
The text-wrap property should be white-space or word-wrap. The CSS property text-wrap is part of the CSS Text Module Level 4, which may not be fully supported across all browsers. Consider using word-wrap: break-word; or overflow-wrap: break-word; for better browser compatibility.
| text-wrap: wrap; | |
| overflow-wrap: break-word; |
There was a problem hiding this comment.
not useful, text-wrap is widely supported:
https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/text-wrap#:~:text=text%2Dwrap%2Dshorthand-,Browser%20compatibility,-Report%20problems%20with
| return ( | ||
| <span> | ||
| {parts.map((part, index) => { |
There was a problem hiding this comment.
The wrapping span element should have a key prop when used as a container for mapped elements, or the map should return a fragment. Since React fragments cannot have keys, consider using React.Fragment with a key or ensure the span serves a semantic purpose.
There was a problem hiding this comment.
I don't know about this, I know items IN the list should have keys, why does the container need a key?
I think it's fine to use a span here, although it's possible a fragment could work. It doesn't need a key either way.
1 participant
Problem
Ongoing mobile work.
Making labels wrap in the mutation table, fixing some spacing and overflow in the clone table.
Solution
Screenshots (optional):