[OSDEV-2375] Reorder and restyle heading elements#901
Merged
VadimKovalenkoSNF merged 4 commits intomainfrom Mar 5, 2026
Merged
[OSDEV-2375] Reorder and restyle heading elements#901VadimKovalenkoSNF merged 4 commits intomainfrom
VadimKovalenkoSNF merged 4 commits intomainfrom
Conversation
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Error
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Error
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 41.45%Your code coverage diff: 0.02% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 55.73%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
📝 WalkthroughWalkthroughExtracts OS ID UI from LocationTitle into a new OsIdBadge component, removes OS ID rendering and related styles from LocationTitle, integrates OsIdBadge into ProductionLocationDetailsContent, adjusts spacing and styles, and moves OS ID tests into a new OsIdBadge test file. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as OsIdBadge (component)
participant Clipboard as CopyToClipboard / CopySearch
participant Toast as toast (react-toastify)
participant Dialog as DialogTooltip
rect rgba(30,144,255,0.5)
User->>UI: Click "Copy OS ID"
UI->>Clipboard: copy(osId)
Clipboard-->>UI: copy success
UI->>Toast: show("Copied OS ID")
Toast-->>User: visible notification
end
rect rgba(34,139,34,0.5)
User->>UI: Click "Learn more" info button
UI->>Dialog: open(url: OS_ID_LEARN_MORE_URL)
Dialog-->>User: render dialog with link
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx (1)
34-34: Minor cleanup: remove redundant fallback.Line 34 already provides a default via
get(..., ''); trailing|| ''is unnecessary.♻️ Proposed refactor
- const osId = get(data, 'properties.os_id', '') || ''; + const osId = get(data, 'properties.os_id', '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx` at line 34, The assignment to osId uses get(data, 'properties.os_id', '') which already supplies a default, so remove the redundant fallback "|| ''" in the expression that defines osId (the const osId declaration referencing get and 'properties.os_id') to simplify the line; ensure the resulting line keeps the same behavior by relying solely on get's default.src/react/src/components/ProductionLocation/Heading/osIdBadge/styles.js (1)
16-16: Use the localspacingfallback consistently.Line 16 bypasses the
spacingvariable and directly usestheme.spacing.unit; this weakens the fallback introduced on Line 6.♻️ Proposed refactor
- marginBottom: theme.spacing.unit * 2, + marginBottom: spacing * 2,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/Heading/osIdBadge/styles.js` at line 16, The marginBottom declaration currently uses theme.spacing.unit directly, bypassing the local spacing fallback; update the styles object so marginBottom uses the existing spacing variable (i.e., replace theme.spacing.unit * 2 with spacing * 2) to consistently honor the fallback introduced earlier (refer to the spacing variable and the marginBottom property in styles.js).src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/styles.js (1)
4-6: Prefer theme spacing token over hardcoded40px.Using
theme.spacing.unit * 5here keeps spacing consistent if the global spacing scale changes.♻️ Proposed refactor
container: Object.freeze({ [theme.breakpoints.up('md')]: { - paddingLeft: '40px', + paddingLeft: theme.spacing.unit * 5, }, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/styles.js` around lines 4 - 6, Replace the hardcoded paddingLeft value inside the ProductionLocationDetailsContent styles where you have [theme.breakpoints.up('md')]: { paddingLeft: '40px' } with the theme spacing token (e.g. paddingLeft: theme.spacing.unit * 5 or paddingLeft: theme.spacing(5) depending on our MUI version) so the component uses the global spacing scale; update the style definition in the same styles module where the breakpoint rule is declared.src/react/src/__tests__/components/LocationTitle.test.jsx (1)
37-45: UsetoBeEmptyDOMElement()for clarity, not becausetoHaveTextContent('')is non-assertive.Jest-dom special-cases
toHaveTextContent('')and explicitly fails when the element has non-empty text content. The matcher is already assertive. However, jest-dom's own documentation recommendstoBeEmptyDOMElement()for checking truly empty DOM elements, which is more explicit about intent.Suggested alternative
- expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent(''); + expect(screen.getByRole('heading', { level: 1 })).toBeEmptyDOMElement();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/src/__tests__/components/LocationTitle.test.jsx` around lines 37 - 45, Replace the assertion that checks for an empty heading in the LocationTitle test: instead of asserting that the heading has empty text via screen.getByRole('heading', { level: 1 }) toHaveTextContent(''), call toBeEmptyDOMElement() on that same queried element; update the expectation in the test that uses renderLocationTitle({ data }) so it asserts emptiness with toBeEmptyDOMElement() (keep the same getByRole call and the surrounding test setup).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/react/src/components/ProductionLocation/Heading/osIdBadge/OsIdBadge.jsx`:
- Around line 20-27: The wrapper element around the Typography h2
(className=osIdValueWithTooltip) uses a <span>, which is invalid because <h2>
(heading content) cannot be a child of phrasing content; replace the span
wrapper with a block-level container (e.g., a div) for both occurrences in
OsIdBadge.jsx (the element that wraps the Typography with
className=osIdValueWithTooltip and the second occurrence around line 54) so the
Typography (h2) is nested inside a block container while preserving className
and styling.
---
Nitpick comments:
In `@src/react/src/__tests__/components/LocationTitle.test.jsx`:
- Around line 37-45: Replace the assertion that checks for an empty heading in
the LocationTitle test: instead of asserting that the heading has empty text via
screen.getByRole('heading', { level: 1 }) toHaveTextContent(''), call
toBeEmptyDOMElement() on that same queried element; update the expectation in
the test that uses renderLocationTitle({ data }) so it asserts emptiness with
toBeEmptyDOMElement() (keep the same getByRole call and the surrounding test
setup).
In `@src/react/src/components/ProductionLocation/Heading/osIdBadge/styles.js`:
- Line 16: The marginBottom declaration currently uses theme.spacing.unit
directly, bypassing the local spacing fallback; update the styles object so
marginBottom uses the existing spacing variable (i.e., replace
theme.spacing.unit * 2 with spacing * 2) to consistently honor the fallback
introduced earlier (refer to the spacing variable and the marginBottom property
in styles.js).
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsx`:
- Line 34: The assignment to osId uses get(data, 'properties.os_id', '') which
already supplies a default, so remove the redundant fallback "|| ''" in the
expression that defines osId (the const osId declaration referencing get and
'properties.os_id') to simplify the line; ensure the resulting line keeps the
same behavior by relying solely on get's default.
In
`@src/react/src/components/ProductionLocation/ProductionLocationDetailsContent/styles.js`:
- Around line 4-6: Replace the hardcoded paddingLeft value inside the
ProductionLocationDetailsContent styles where you have
[theme.breakpoints.up('md')]: { paddingLeft: '40px' } with the theme spacing
token (e.g. paddingLeft: theme.spacing.unit * 5 or paddingLeft: theme.spacing(5)
depending on our MUI version) so the component uses the global spacing scale;
update the style definition in the same styles module where the breakpoint rule
is declared.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8d757748-dc71-44e9-bb21-9ce9276fbf77
📒 Files selected for processing (12)
src/react/src/__tests__/components/LocationTitle.test.jsxsrc/react/src/__tests__/components/OsIdBadge.test.jsxsrc/react/src/components/ProductionLocation/Heading/ClaimFlag/styles.jssrc/react/src/components/ProductionLocation/Heading/ClosureStatus/styles.jssrc/react/src/components/ProductionLocation/Heading/LocationTitle/LocationTitle.jsxsrc/react/src/components/ProductionLocation/Heading/LocationTitle/styles.jssrc/react/src/components/ProductionLocation/Heading/osIdBadge/OsIdBadge.jsxsrc/react/src/components/ProductionLocation/Heading/osIdBadge/constants.jssrc/react/src/components/ProductionLocation/Heading/osIdBadge/styles.jssrc/react/src/components/ProductionLocation/ProductionLocationDetailsContainer/ProductionLocationDetailsContainer.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsContent/ProductionLocationDetailsContent.jsxsrc/react/src/components/ProductionLocation/ProductionLocationDetailsContent/styles.js
src/react/src/components/ProductionLocation/Heading/osIdBadge/OsIdBadge.jsx
Outdated
Show resolved
Hide resolved
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Error
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Error
VadimKovalenkoSNF
had a problem deploying
to
Quality Environment
GitHub Actions
Error
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
|
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.75%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Django App | Unittest test suite - Code coverage reportTotal: 81.97%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
vlad-shapik
approved these changes
Mar 5, 2026
This was referenced Mar 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[PL Redesign]: Reorder and restyle heading elements.
Follow-up PR for OSDEV-2375