Skip to content

Conversation

@eweren
Copy link
Contributor

@eweren eweren commented Sep 16, 2025

The devmode now allows to push keys and see + edit unconnected keys. This should allow developers to have more possibilities to interact with the figma files and make easier integration in the dev workflow

Summary by CodeRabbit

  • New Features

    • Top bar is now always visible; controls appear based on mode and available languages.
    • Resize handle is hidden in developer mode.
    • Node lists consistently use the full selection regardless of mode.
  • Style

    • Improved namespace cell styling for clearer layout.
    • Standardized row height (28px) in change lists for consistent spacing.
    • “Create a copy” button label shortened to “Copy” and moved into the top bar.
    • Language dropdown now appears only when languages exist and not in developer mode.
    • Pull button shows only outside developer mode.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 16, 2025

Walkthrough

Adds a new .ns style to NodeRow and applies it in the namespace element. Adjusts TopBar rendering and control visibility in Index based on editor mode and languages. Sets fixed rowHeight for NodeList in Push/Changes. In Router, conditionally renders ResizeHandle using editorMode.

Changes

Cohort / File(s) Summary of modifications
NodeRow namespace styling
src/ui/components/NodeList/NodeRow.css, src/ui/components/NodeList/NodeRow.css.d.ts, src/ui/components/NodeList/NodeRow.tsx
Added .ns CSS class (cursor: unset; grid-column: 1 / span 2); updated CSS module typings with ns; applied className={styles.ns} to namespace div in NodeRow.
Index TopBar conditional structure
src/ui/views/Index/Index.tsx
TopBar always rendered; inner controls now conditionally shown based on editorMode.data and languages. Language dropdown and Pull button hidden in dev mode; Copy label changed to “Copy”; NodeList uses full selection without dev/non-dev filtering.
Push view NodeList sizing
src/ui/views/Push/Changes.tsx
Added rowHeight={28} to two NodeList instances (New keys, Changed keys).
Router editor-mode dependent UI
src/ui/views/Router.tsx
Imported and used useEditorMode; ResizeHandle rendered only when editorMode.data !== "dev".

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Router
  participant EditorMode as useEditorMode
  participant UI as ResizeHandle

  User->>Router: Navigate app
  Router->>EditorMode: get editorMode
  EditorMode-->>Router: editorMode.data
  alt editorMode != "dev"
    Router->>UI: Render ResizeHandle
  else editorMode == "dev"
    Router--x UI: Do not render ResizeHandle
  end
Loading
sequenceDiagram
  participant User
  participant Index as Index View
  participant EditorMode as useEditorMode
  participant TopBar
  participant NodeList

  User->>Index: Open project
  Index->>EditorMode: get editorMode
  EditorMode-->>Index: editorMode.data
  Index->>TopBar: Render TopBar (always)
  alt editorMode != "dev"
    TopBar-->>User: Show Language dropdown, Pull button
  else editorMode == "dev"
    TopBar-->>User: Hide Language dropdown, Pull button
  end
  Index->>NodeList: Render list (full selection)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • stepan662
  • JanCizmar

Poem

A nibble of CSS, a hop through TSX,
I groomed the TopBar—less ifs, fewer checks.
In dev I hide, in prod I show,
With rows at 28, steady in flow.
Resize? Only when it’s not dev—
Thump-thump! Ship it, I’m ready to rev. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Adjustments to devmode" is concise, single-line, and correctly identifies the primary area of change (devmode behavior), which matches the PR objectives such as allowing pushes and editing unconnected keys; it therefore reflects the main intent of the changeset. Although somewhat generic, it is still informative enough for a teammate scanning commit history to understand the high-level purpose.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (6)
src/ui/components/NodeList/NodeRow.css (1)

39-42: Deduplicate .key and .ns rules; also make .action span all rows to avoid misalignment.

.ns duplicates .key. Combine them. With the new third row, .action currently spans only two rows; center it across all rows to prevent odd alignment when namespace is present.

Apply:

 .action {
-  grid-column: 3;
-  grid-row: 1 / span 2;
+  grid-column: 3;
+  grid-row: 1 / -1;
   align-self: center;
 }
 
-.key {
-  cursor: unset;
-  grid-column: 1 / span 2;
-}
-
-.ns {
-  cursor: unset;
-  grid-column: 1 / span 2;
-}
+.key,
+.ns {
+  cursor: unset;
+  grid-column: 1 / span 2;
+}
src/ui/views/Push/Changes.tsx (1)

28-28: Verify fixed rowHeight=28 vs variable content height (possible clipping/overlap).

NodeRow can render text, key, and namespace; content may exceed 28px, especially with wrapped text. Confirm in both “New keys” and “Changed keys” lists that virtualization (if any) doesn’t clip.

If clipping occurs, consider:

  • Use a taller rowHeight (e.g., 48–56), or
  • Provide a custom row that omits/wraps less, or
  • Switch to dynamic measurement (estimateSize) if NodeList supports it.

Also applies to: 51-51

src/ui/views/Router.tsx (1)

103-104: Avoid dev/non‑dev flicker while editorMode loads.

editorMode.data is undefined initially, so the handle renders then disappears in dev, causing flicker.

Apply:

-      {editorMode.data !== "dev" && <ResizeHandle />}
+      {editorMode.isSuccess && editorMode.data !== "dev" && <ResizeHandle />}
src/ui/views/Index/Index.tsx (3)

120-137: Gate UI by editorMode load state to prevent flicker.

Same pattern as Router: undefined editorMode.data evaluates as non‑dev. Wait for success before showing non‑dev controls.

Minimal change:

-                {languages && editorMode.data !== "dev" && (
+                {languages && editorMode.isSuccess && editorMode.data !== "dev" && (
                   <select
                     data-cy="index_language_select"
                     className={styles.languageContainer}
                     value={language}
                     onChange={(e) => {
                       handleLanguageChange(
                         (e.target as HTMLInputElement).value
                       );
                     }}
                   >
                     {languages.map((l) => (
                       <option key={l.tag} value={l.tag}>
                         {l.name}
                       </option>
                     ))}
                   </select>
                 )}
...
-                {editorMode.data !== "dev" && (
+                {editorMode.isSuccess && editorMode.data !== "dev" && (
                   <Fragment>
                     <Button
                       data-cy="index_pull_button"
                       onClick={handlePull}
                       secondary
                     >
                       {nothingSelected ? "Pull all" : "Pull"}
                     </Button>
                     <Button
                       data-cy="index_create_copy_button"
                       onClick={handleCopy}
                       secondary
                     >
                       Copy
                     </Button>
                   </Fragment>
                 )}

Also applies to: 143-161


121-129: Add an accessible label to the language select.

The select lacks an associated label; add an aria-label for screen readers.

-                  <select
+                  <select
+                    aria-label="Language"
                     data-cy="index_language_select"
                     className={styles.languageContainer}
                     value={language}
                     onChange={(e) => {

165-175: Make the Settings “button” keyboard accessible.

It’s a div with role=button but no keyboard handling or focusability.

-                <div
+                <div
                   data-cy="index_settings_button"
                   className={styles.settingsButton}
                   onClick={() => setRoute("settings")}
                   role="button"
+                  tabIndex={0}
+                  onKeyDown={(e) => {
+                    if (e.key === "Enter" || e.key === " ") {
+                      e.preventDefault();
+                      setRoute("settings");
+                    }
+                  }}
                 >
                   <Settings width={15} height={15} />
                 </div>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee19b8 and b8995f7.

📒 Files selected for processing (6)
  • src/ui/components/NodeList/NodeRow.css (1 hunks)
  • src/ui/components/NodeList/NodeRow.css.d.ts (1 hunks)
  • src/ui/components/NodeList/NodeRow.tsx (1 hunks)
  • src/ui/views/Index/Index.tsx (2 hunks)
  • src/ui/views/Push/Changes.tsx (2 hunks)
  • src/ui/views/Router.tsx (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/ui/views/Router.tsx (2)
src/ui/hooks/useEditorMode.ts (1)
  • useEditorMode (4-9)
src/ui/components/ResizeHandle/ResizeHandle.tsx (1)
  • ResizeHandle (15-154)
src/ui/views/Index/Index.tsx (2)
src/ui/components/TopBar/TopBar.tsx (1)
  • TopBar (12-31)
src/ui/state/GlobalState.ts (1)
  • setRoute (61-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cypress
🔇 Additional comments (3)
src/ui/components/NodeList/NodeRow.css.d.ts (1)

6-6: Type addition looks good.

styles.ns matches the CSS module and usage in TSX.

src/ui/components/NodeList/NodeRow.tsx (1)

80-80: LGTM: style hook-up for namespace.

Wires the new .ns class without changing behavior.

src/ui/views/Index/Index.tsx (1)

205-205: Confirm NodeList row height vs content here too.

Selection rows may show long text and namespace; ensure default NodeList rowHeight (or lack thereof) matches actual content height to avoid clipping, consistent with the Push/Changes view.

@JanCizmar JanCizmar merged commit aaa074a into tolgee:main Sep 17, 2025
4 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants