Test Case Details Page content #4403
Conversation
Guria
requested review from
AmsterGet,
BlazarQSO,
maria-hambardzumian and
allaprischepa
as code owners
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces and refactors several UI components for the test case library pages, including a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestCaseDetailsPage
participant TestCaseDetailsHeader
participant Sidebar (EditableTagsSection/EditableDescriptionSection)
participant MainContent (DetailsEmptyState)
User->>TestCaseDetailsPage: Visit page
TestCaseDetailsPage->>TestCaseDetailsHeader: Render header with test case data
TestCaseDetailsPage->>Sidebar: Render tags and description sections
TestCaseDetailsPage->>MainContent: Render DetailsEmptyState
User->>Sidebar: Click "Add Tag" or "Add Description"
Sidebar->>Sidebar: Trigger onAddTag/onAddDescription callback
User->>TestCaseDetailsHeader: Use menu actions (duplicate, history, delete)
TestCaseDetailsHeader->>TestCaseDetailsHeader: Handle menu action callback
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/test-library #4403 +/- ##
=====================================================
Coverage 66.73% 66.73%
=====================================================
Files 85 85
Lines 995 995
Branches 143 143
=====================================================
Hits 664 664
Misses 300 300
Partials 31 31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.scss (1)
18-23: 🛠️ Refactor suggestionAvoid global CSS overrides when possible.
Applying:global(.width-content)may unintentionally affect other components; consider scoping this workaround within thepopover-toolscontainer or targeting a more specific selector until the UI-kit fix lands.app/src/pages/common/popoverControl/popoverControl.tsx (1)
54-66:⚠️ Potential issue
labelused as React key → collisions possible
labelis translated text; two items could share the same string causing duplicate-key warnings.
Useindexor a stable id instead.- {items.map(({ label, icon, className = '', onClick, variant }) => ( - <li key={label}> + {items.map(({ label, icon, className = '', onClick, variant }, idx) => ( + <li key={idx}>
🧹 Nitpick comments (20)
app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.tsx (1)
17-31: Expose standard HTML props & type the component
InfoBlockcurrently swallows common div props (id, style, onClick, etc.).
ExtendingReact.HTMLAttributes<HTMLDivElement>keeps it flexible with zero runtime cost.-interface InfoBlockProps { - label: string; - className?: string; -} +interface InfoBlockProps extends React.HTMLAttributes<HTMLDivElement> { + label: string; + className?: string; +} -export const InfoBlock = ({ label, className = '' }: InfoBlockProps) => ( - <div className={cx('info-block', className)}> +export const InfoBlock: React.FC<InfoBlockProps> = ({ label, className = '', ...rest }) => ( + <div className={cx('info-block', className)} {...rest}> <span className={cx('info-block__label')}>{label}</span> </div> );app/src/common/constants/localization.js (1)
198-201: Ensure translations are updated.
You’ve added a newDUPLICATEkey—run your message extraction and update all language packs to include this string.app/src/pages/inside/testCaseLibraryPage/emptyState/emptyStateLibrary.tsx (2)
35-45: MemoisehandleCreateTestCaseto avoid needless re-renders
Each render creates a new callback which will trigger prop inequality in theEmptyStatePagebuttons array. Wrap it inuseCallback.-import { useDispatch, useSelector } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; +import { useCallback } from 'react'; @@ - const handleCreateTestCase = () => { + const handleCreateTestCase = useCallback(() => { dispatch({ type: TEST_CASE_DETAILS_PAGE, payload: { // temporary - will be replaced with actual ID generation testCaseSlug: 'new', organizationSlug, projectSlug, }, }); - }; + }, [dispatch, organizationSlug, projectSlug]);
47-53: Minor perf nit – derivebenefitsonce
benefitsis recomputed on every render. Not critical, but wrapping the map inuseMemowould prevent unnecessary work/XSS parsing.app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.jsx (1)
102-103: Stale CSS-module key:popover-toolsno longer existsSCSS cleanup removed the
.popover-toolsrule.
Referencing a non-existent key in a CSS-module returnsundefined, so the resultingclassNameis an empty string and the attribute is useless.- className={cx('popover-tools')} + // className removed – wrapper inherits PopoverControl defaultsapp/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/messages.ts (1)
36-40: Rename the “id” message key to avoid self-shadowingUsing
idas an object key insidedefineMessagesis legal but highly confusing because each message descriptor also contains anidfield.
Consider renaming the outer key toidLabel(or similar) for clarity:- id: { + idLabel: { id: 'EditTestCasePage.id', defaultMessage: 'ID:', },app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.tsx (2)
24-29: Allow ReactNode fortitleto support localisation markup
titleis typed asstring; many translated headers areFormattedMessagecomponents (ReactNode).
Changing the prop toReactNodeprovides flexibility with zero breaking impact:- title: string; + title: ReactNode;
38-45: Semantic HTML / a11yA
<h3>will be rendered no matter where this component is nested. If the consumer places it inside another<h3>you’ll get heading-level skips.
Consider accepting aheadingLevel?: 2|3|4|...prop or delegating the heading tag to the parent.app/src/pages/common/breadcrumbs/breadcrumbs.tsx (1)
41-57: Accessibility: use<nav aria-label="Breadcrumb">Wrapping the breadcrumbs in a
<nav>element with anaria-labelimproves assistive-tech navigation.- return ( - <div className={cx('breadcrumbs', className)}> + return ( + <nav aria-label="Breadcrumb" className={cx('breadcrumbs', className)}> ... - </div> + </nav>app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.scss (1)
25-27: Global override belongs in a shared util, not a local component stylesheetOverriding
.width-contenthere couples unrelated styles to this component and will be hard to track once the workaround is removed. Prefer moving such cross-cutting overrides to a dedicated global “workarounds” (or ui-kit-patches) file and add a TODO referencing the upstream issue.app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.jsx (1)
37-38: CachebreadcrumbItemswithuseMemoThe array is recreated on every render, forcing unnecessary re-renders of
Breadcrumbs.-import { Breadcrumbs } from 'pages/common'; +import React, { useMemo } from 'react'; +import { Breadcrumbs } from 'pages/common'; ... - const breadcrumbItems = [formatMessage(messages.productVersions), productVersionId]; + const breadcrumbItems = useMemo( + () => [formatMessage(messages.productVersions), productVersionId], + [formatMessage, productVersionId], + );app/src/pages/inside/testCaseLibraryPage/editableTagsSection/editableTagsSection.tsx (1)
34-37: Add an explicit aria-label for accessibilityIcon-only buttons rely on screen-reader text inside. Because the Plus icon conveys no textual meaning, add
aria-label(ortitle) to theButton.- <Button onClick={onAddTag} variant="text"> + <Button onClick={onAddTag} variant="text" aria-label={formatMessage(COMMON_LOCALE_KEYS.ADD)}>app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.tsx (1)
58-62: NestedScrollWrappers may lead to double scroll barsThere is an outer wrapper for the whole page and another inside the main-content area. Unless required for independent scrolling regions, one wrapper should be enough.
app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/editableDescriptionSection.tsx (1)
30-52: Minor: memoise headerControl to avoid needless re-creates
headerControlis re-computed on every render even though it is purely static.
Not critical here, but a quickuseMemokeeps the component flicker-free if you later add animations.const headerControl = useMemo( () => ( <Button onClick={onAddDescription} variant="text"> <PlusIcon /> {formatMessage(COMMON_LOCALE_KEYS.ADD)} </Button> ), [onAddDescription, formatMessage], );app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.scss (2)
17-37: Fixed 360 px sidebar may hurt smaller viewportsThe sidebar is hard-coded to
360px. On narrow laptops (<1280 px) this leaves <900 px for content and may trigger horizontal scroll.Consider switching to a css-var or
minmax(280px, 360px)to let the sidebar shrink gracefully.- / 360px 1fr; + / minmax(280px, 360px) 1fr;
39-46: Nit: 1 px margin used as layout fix
margin-left: 1px;is commonly added to hide the right border overlap, but it propagates into nested flex math.Safer alternative: move the right border from the sidebar to a pseudo-element or apply
border-leftto.page__main-content.app/src/pages/common/popoverControl/popoverControl.tsx (2)
58-61: Edge-case: class name produced whenvariantis undefinedTemplate literal is evaluated even when
variantis falsy, yielding
popover-control__item-button--undefined(later stripped by classNames but still odd).- { [`popover-control__item-button--${variant}`]: !!variant } + { ...(variant ? { [`popover-control__item-button--${variant}`]: true } : {}) }
26-32: Consider wideningvariantunion for future-proofingLimiting to
'danger'forces another breaking change when we need e.g.'success'.
You can type-safe yet flexible variants:type ButtonVariant = 'danger' | 'warning' | 'info'; variant?: ButtonVariant;app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx (2)
74-76: Accessibility: addaria-labelto the copy iconScreen-reader users have no context for the duplicate icon.
<CopyToClipboard text={testCase.id} className={cx('header__copy')}> - {Parser(IconDuplicate)} + {Parser(IconDuplicate)} + <span className="visually-hidden">{formatMessage(COMMON_LOCALE_KEYS.COPY)}</span> </CopyToClipboard>
56-57: MemoisebreadcrumbItemsRe-allocating the array on every render causes needless re-renders of
Breadcrumbs.const breadcrumbItems = useMemo( () => [formatMessage(messages.testCaseLibraryBreadcrumb), testCase.name], [formatMessage, testCase.name], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
app/src/common/constants/localization.js(1 hunks)app/src/pages/common/breadcrumbs/breadcrumbs.scss(1 hunks)app/src/pages/common/breadcrumbs/breadcrumbs.tsx(1 hunks)app/src/pages/common/breadcrumbs/index.ts(1 hunks)app/src/pages/common/index.ts(2 hunks)app/src/pages/common/popoverControl/popoverControl.scss(1 hunks)app/src/pages/common/popoverControl/popoverControl.tsx(2 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.jsx(1 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.scss(1 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.jsx(2 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/editableDescriptionSection.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableTagsSection/editableTagsSection.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableTagsSection/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/emptyStateDetails.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/emptyStateLibrary.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/infoBlock/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.scss(2 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.tsx(2 hunks)app/src/types.d.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
app/src/pages/common/breadcrumbs/breadcrumbs.tsx (2)
app/src/pages/common/breadcrumbs/index.ts (1)
Breadcrumbs(17-17)app/src/pages/common/index.ts (1)
Breadcrumbs(19-19)
app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/messages.ts (1)
app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/messages.ts (1)
messages(19-36)
app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/editableDescriptionSection.tsx (4)
app/src/common/constants/localization.js (2)
COMMON_LOCALE_KEYS(25-322)COMMON_LOCALE_KEYS(25-322)app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.tsx (1)
SectionWithHeader(31-46)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/messages.ts (1)
messages(19-36)app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.tsx (1)
InfoBlock(27-31)
app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.tsx (1)
app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/index.ts (1)
SectionWithHeader(17-17)
app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx (4)
app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/messages.ts (1)
messages(19-44)app/src/pages/common/breadcrumbs/breadcrumbs.tsx (1)
Breadcrumbs(36-58)app/src/pages/common/popoverControl/popoverControl.tsx (1)
PopoverControl(42-74)app/src/common/constants/localization.js (2)
COMMON_LOCALE_KEYS(25-322)COMMON_LOCALE_KEYS(25-322)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Call feature Docker build / Build and export to AWS ECR
🔇 Additional comments (22)
app/src/pages/common/popoverControl/popoverControl.scss (1)
44-52: 👍 Consistent “danger” variant styling addedClean BEM-style modifier, variables reused, hover/focus states covered. Looks good.
app/src/pages/inside/testCaseLibraryPage/emptyState/messages.ts (1)
56-68: Message IDs out of namespace – confirm placementAll other keys in this file follow
TestCaseLibraryPage.*, but the three new entries useEditTestCasePage.*.
If these strings truly belong to a different page, consider moving them to that page’s messages file to avoid accidental bundle bloat and easier grep-ability.app/src/pages/common/index.ts (1)
17-20: Export list updated – check tree-shaking side-effectsAdding
Breadcrumbshere is correct, just ensure the new component re-export (./breadcrumbs/index.ts) is barrel-safe (no side-effects) so that unused locales don’t get bundled inadvertently.app/src/pages/common/breadcrumbs/index.ts (1)
17-17: Re-export looks good.
This index file correctly re-exports theBreadcrumbscomponent, enabling simplified imports across the app.app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.scss (1)
1-37: Styles are well-scoped and follow BEM conventions.
TheSectionWithHeaderstylesheet is comprehensive, uses CSS modules to avoid collisions, and adheres to your naming standards. No issues found.app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.scss (1)
21-23: Verify breadcrumb spacing with the new component.
Since custom tree-icon and name styles were removed and onlymargin-bottom: 12pxremains, confirm that the newBreadcrumbscomponent’s native spacing aligns with the design system before shipping.app/src/pages/inside/testCaseLibraryPage/infoBlock/index.ts (1)
17-17: Barrel file looks goodSingle-line re-export is consistent with the new component structure and keeps import paths clean.
app/src/pages/inside/testCaseLibraryPage/editableTagsSection/index.ts (1)
17-17: Consistent export pattern – LGTMMaintains the same barrel-export convention as other new sections, aiding discoverability.
app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/index.ts (1)
17-17: Export looks fineNo issues spotted.
app/src/pages/inside/testCaseLibraryPage/emptyState/emptyStateLibrary.tsx (1)
36-44: Placeholder slug could leak to prod
testCaseSlug: 'new'is marked temporary, but if it ships it’ll collide for every new test-case creation. Please ensure this is replaced by a generated UUID / server-side id before merge or guard it behind a feature flag.app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/index.ts (1)
17-17: Confirm the underlying component’s export form
export { TestCaseDetailsHeader }assumes thattestCaseDetailsHeader.tsxcontains a named export.
If that file instead uses a default export (export default TestCaseDetailsHeader), consumers will getundefined.-export { TestCaseDetailsHeader } from './testCaseDetailsHeader'; +// Use the form that matches the implementation file +export { default as TestCaseDetailsHeader } from './testCaseDetailsHeader';Double-check the implementation file and align the barrel export accordingly.
app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/index.ts (1)
17-17: Verify barrel export directionSame caution as above: ensure the underlying component is exported as a named export; otherwise switch to
default as EditableDescriptionSection.app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.jsx (2)
109-112: EnsurePopoverControlsupportsvariant: "danger"The new
variantprop will only style the delete action if thePopoverControlcomponent passes it through to the UI-kitButton(or adds its own styling).
If that support was not added in the same PR, the prop is silently ignored.Please confirm
PopoverControlhandles thevariantfield for menu items; otherwise the button will render with default styling and we lose the intended visual warning.
116-118:adjustWidthOn="content"is UI-kit-specific – confirm compatibility
@reportportal/ui-kit’sButtondid not expose anadjustWidthOnprop in earlier versions. Passing an unknown prop will be forwarded to the DOM, causing a React warning.Verify that the upgraded UI-kit version indeed introduces this prop; if not, replace with the previous CSS workaround.
app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.scss (1)
26-29: Good consolidation of header stylingSwitching to CSS variables and adding a bottom border improves consistency with other pages.
app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.scss (1)
26-32: Verify that$FONT-ROBOTO-REGULARis globally availableThe stylesheet references the Sass variable
$FONT-ROBOTO-REGULAR, but no local import is made. If this file is compiled in isolation (e.g., via CSS-Modules that don’t share the global namespace), the build will fail.Diff suggestion if a local import is required:
+@import 'styles/typography'; // path to the file that defines $FONT-ROBOTO-REGULARapp/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.tsx (1)
41-47: Minor: className prop can be pushed downYou pass
className={cx('test-case-library-page__breadcrumb')}into<Breadcrumbs/>.
The component already acceptsclassName, so this is fine; just ensure the SCSS file defines that modifier, otherwise it’s a no-op.
Nothing to change – just a reminder.app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.scss (1)
36-38: Verify that$FONT-*variables are injected automaticallyThis file relies on
$FONT-SEMIBOLDand$FONT-ROBOTO-REGULAR, but no@use/@importis present.
If the build no longer injects global typography variables by default, SASS compilation will fail. Please confirm the loader config or add an explicit import (e.g.@use 'styles/variables/fonts';).Also applies to: 64-88
app/src/pages/common/breadcrumbs/breadcrumbs.scss (1)
21-23: Same global-variable concern applies here
$FONT-ROBOTO-REGULARis referenced without an explicit import. Make sure the variable is available in the module scope or add the import.app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.jsx (1)
41-42: ConfirmBreadcrumbsaccepts a plain string arrayThe new component is passed an array of strings, but some breadcrumb APIs expect
{ title, href }objects. Double-check the prop‐types to avoid runtime warnings.app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/editableDescriptionSection.tsx (1)
36-41: Confirm thatvariant="text"is supported by UI-KitMost buttons in the codebase use variants such as
primary,ghost, etc.
Iftextis not a valid variant in the currently installed@reportportal/ui-kit, the button will silently fall back to the default style.- <Button onClick={onAddDescription} variant="text"> + <Button onClick={onAddDescription} variant="ghost"> // or another valid variantPlease double-check the available variants in the design system and adjust if necessary.
app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/messages.ts (1)
19-36: LGTM – solid message extractionMessage IDs follow the existing
EditTestCasePage.*namespace and default strings are concise.
app/src/pages/inside/testCaseLibraryPage/emptyState/emptyStateDetails.tsx
Outdated
Show resolved
Hide resolved
app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.tsx
Outdated
Show resolved
Hide resolved
...side/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx
Show resolved
Hide resolved
...side/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx
Outdated
Show resolved
Hide resolved
...side/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx
Outdated
Show resolved
Hide resolved
Guria
force-pushed
the
feature/edit-test-case
branch
from
9f8fec8 to
e1a5122
Compare
app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.scss
Show resolved
Hide resolved
...side/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx
Show resolved
Hide resolved
|
I see that tags and description are collapsible per design. Is it no longer the requirement? |
Guria
force-pushed
the
feature/edit-test-case
branch
from
e1a5122 to
a705946
Compare
|
@ViktorSoroka07 I've already decomposed, extracted and implemented several tasks here. I think we might need separate task for collapsible |
Guria
force-pushed
the
feature/edit-test-case
branch
from
a705946 to
6c88bab
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/pages/inside/testCaseLibraryPage/emptyState/details/detailsEmptyState.tsx (1)
17-18: Minor: avoid misleading import alias
html-react-parserexports a plain function; importing it asParser(PascalCase) suggests a constructor/class.
Consider:-import Parser from 'html-react-parser'; +import parse from 'html-react-parser'; … description={parse(formatMessage(messages.scenarioDescription))}app/src/pages/inside/testCaseLibraryPage/emptyState/mainPage/mainPageEmptyState.tsx (1)
31-46: Tiny optimisation: wraphandleCreateTestCaseinuseCallbackThe handler is recreated on every render, causing needless re-renders of children receiving it as a prop (e.g.,
EmptyStatePage). Not critical but easy to tidy up.- const handleCreateTestCase = () => { + const handleCreateTestCase = useCallback(() => { dispatch({ … }); - }; + }, [dispatch, organizationSlug, projectSlug]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
app/src/common/constants/localization.js(1 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.jsx(2 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.scss(1 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.jsx(2 hunks)app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/editableDescriptionSection.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableTagsSection/editableTagsSection.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/editableTagsSection/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/details/detailsEmptyState.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/details/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/mainPage/mainPageEmptyState.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/emptyState/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/infoBlock/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/index.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.scss(2 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.tsx(2 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseList/priorityIcon/priorityIcon.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/testCaseList/testCaseExecutionCell/testCaseExecutionCell.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- app/src/pages/inside/testCaseLibraryPage/testCaseList/testCaseExecutionCell/testCaseExecutionCell.tsx
- app/src/pages/inside/testCaseLibraryPage/testCaseList/priorityIcon/priorityIcon.tsx
- app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.jsx
- app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/index.ts
- app/src/pages/inside/testCaseLibraryPage/emptyState/details/index.ts
- app/src/common/constants/localization.js
- app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.scss
- app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/index.ts
- app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (19)
- app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.scss
- app/src/pages/inside/productVersionPage/productVersionHeader/popoverTools/popoverTools.scss
- app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/index.ts
- app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.scss
- app/src/pages/inside/testCaseLibraryPage/editableTagsSection/index.ts
- app/src/pages/inside/testCaseLibraryPage/infoBlock/index.ts
- app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.tsx
- app/src/pages/inside/testCaseLibraryPage/editableDescriptionSection/editableDescriptionSection.tsx
- app/src/pages/inside/testCaseLibraryPage/sectionWithHeader/sectionWithHeader.tsx
- app/src/pages/inside/productVersionPage/productVersionHeader/productVersionHeader.jsx
- app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/messages.ts
- app/src/pages/inside/testCaseLibraryPage/editableTagsSection/editableTagsSection.tsx
- app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsPage.scss
- app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.tsx
- app/src/pages/inside/testCaseLibraryPage/emptyState/messages.ts
- app/src/pages/inside/testCaseLibraryPage/infoBlock/infoBlock.tsx
- app/src/pages/inside/testCaseLibraryPage/testCaseLibraryPage.scss
- app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/testCaseDetailsHeader/testCaseDetailsHeader.scss
- app/src/pages/inside/testCaseLibraryPage/testCaseDetailsPage/messages.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/pages/inside/testCaseLibraryPage/emptyState/details/detailsEmptyState.tsx (2)
app/src/pages/inside/testCaseLibraryPage/emptyState/details/index.ts (1)
DetailsEmptyState(17-17)app/src/pages/inside/testCaseLibraryPage/emptyState/messages.ts (1)
messages(19-78)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Call feature Docker build / Build and export to AWS ECR
- GitHub Check: build (20)
🔇 Additional comments (1)
app/src/pages/inside/testCaseLibraryPage/emptyState/mainPage/mainPageEmptyState.tsx (1)
36-45: Redirect → plain dispatch changes browser-history behaviour; confirm this is intended
redirect(TEST_CASE_DETAILS_PAGE, …)performed a history .replace; the new plain action will push a new entry.
If users navigate back they’ll now return to the empty-state page instead of the previous screen.
Verify the UX requirement; if replacement is still desired dispatch an action withmeta: { method: 'replace' }(redux-first-router convention) or revert toredirect.- dispatch({ + dispatch({ type: TEST_CASE_DETAILS_PAGE, payload: { … }, + // meta: { method: 'replace' }, // ← optional: mimic previous behaviour });
app/src/pages/inside/testCaseLibraryPage/emptyState/details/detailsEmptyState.tsx
Show resolved
Hide resolved
Guria
force-pushed
the
feature/edit-test-case
branch
from
6c88bab to
ed34f42
Compare
|
PR Checklist
feature/test-library)npm run lint) prior to submission? Enable the git hook on commit in your IDE to run it and format the code automatically.npm run build)?devnpm script)manage:translationsscript?Visuals
Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes
Documentation