Make layer tab value nullable

[Kitenite]

Description

Related Issues

Type of Change

• Bug fix
• New feature
• Documentation update
• Release
• Refactor
• Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes

---

Important

Make LayersPanelTabValue nullable and introduce BrandTabValue for managing brand-related tabs in the editor.

• Behavior:
  • Make LayersPanelTabValue nullable in EditorEngine to allow no tab selection.
  • Introduce BrandTabValue to manage brand-related tabs (colors and fonts).
  • Add isLayersPanelLocked to lock/unlock the layers panel in EditorEngine.
• Components:
  • Update FontInput.tsx to set layersPanelTab to BRAND and brandTab to FONTS when adding a new font.
  • Modify ColorPanel and FontPanel to clear brandTab on close.
  • Refactor BrandTab to use brandTab for switching between ColorPanel and FontPanel.
  • Adjust LayersPanel to handle nullable layersPanelTab and lock state.
• Models:
  • Add APPS to LayersPanelTabValue and BrandTabValue enums in models.ts.

^{This description was created by for 12dd2bd. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 8b1462f in 1 minute and 23 seconds

More details

• Looked at 60 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. apps/studio/src/lib/editor/engine/index.ts:41

• Draft comment:
  Consider if initializing _layersPanelTab to null is intentional. Consumers may expect a default tab (like LayersPanelTabValue.LAYERS) to avoid handling null.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

2. apps/studio/src/routes/editor/LayersPanel/index.tsx:25

• Draft comment:
  The initial state for selectedTab is set to null; ensure this aligns with UI expectations as no tab will appear selected by default.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

3. apps/studio/src/lib/editor/engine/index.ts:41

• Draft comment:
  Changed default _layersPanelTab from 'PAGES' to null. Verify that starting with no tab selected is intended.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the PR author to verify their intention, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code. Therefore, it should be removed.

4. apps/studio/src/lib/editor/engine/index.ts:186

• Draft comment:
  The setter for layersPanelTab now accepts null. Ensure downstream consumers properly handle a null value.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the author to ensure that downstream consumers handle a null value, which is a form of asking the author to double-check something. This violates the rule against asking the author to ensure behavior is intended or tested.

5. apps/studio/src/routes/editor/LayersPanel/index.tsx:25

• Draft comment:
  The selectedTab state now initializes to null instead of a default like 'LAYERS'. Confirm that having no default selected tab is intended.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The initial null state appears intentional as part of the component's design. The useEffect hook immediately sets the correct tab from editorEngine on mount, and the component has clear logic for handling both the null and non-null states. This seems like a deliberate pattern rather than an oversight.
  I could be missing some edge case where the editorEngine.layersPanelTab is not yet initialized when the component mounts. There could be a brief flash of an invalid UI state.
  Even if there is a brief flash, this would be a minor UI glitch at worst, and the code suggests this is an intentional design choice given the comprehensive state management logic.
  This comment should be deleted as it questions an intentional design choice that is well-handled by the component's logic.

6. apps/studio/src/routes/editor/LayersPanel/index.tsx:81

• Draft comment:
  useEffect calls tabChange with editorEngine.layersPanelTab, which is initially null. Review whether opening the content panel with a null selected tab yields the desired UI behavior.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_rVtpQtXo72dnwK6m

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 970c9b1 in 1 minute and 1 seconds

More details

• Looked at 169 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 9 drafted comments based on config settings.

1. apps/studio/src/lib/editor/engine/index.ts:48

• Draft comment:
  BrandTab property added. Ensure that the new getter/setter and default value (null) align with expected usage throughout the app. If there are any invariants, consider documenting them.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

2. apps/studio/src/routes/editor/LayersPanel/index.tsx:56

• Draft comment:
  On mouse leave, resetting the tab by assigning 'editorEngine.layersPanelTab = selectedTab' relies on the local 'selectedTab' variable directly reflecting engine state. Confirm that this behavior is as intended, as MobX observability should keep it updated.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

3. apps/studio/src/routes/editor/LayersPanel/index.tsx:197

• Draft comment:
  The removal of the local state for selectedTab and content panel state simplifies the component. Verify that using 'editorEngine.layersPanelTab' directly (via MobX) covers all UI update scenarios without side-effects.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

4. apps/studio/src/lib/editor/engine/index.ts:1

• Draft comment:
  Ensure that BrandTabValue is defined in '@/lib/models' and used consistently with the other tab values.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

5. apps/studio/src/lib/editor/engine/index.ts:48

• Draft comment:
  New _brandTab property with nullable type is implemented similarly to _layersPanelTab. Verify that corresponding model changes (if any) support this.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

6. apps/studio/src/routes/editor/LayersPanel/index.tsx:23

• Draft comment:
  Deriving selectedTab from editorEngine.layersPanelTab is fine, but ensure that its usage in event handlers (e.g. in handleMouseLeave) reflects the latest observable value.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

7. apps/studio/src/routes/editor/LayersPanel/index.tsx:197

• Draft comment:
  The removal of the COMPONENT_DISCOVERY_ENABLED constant and its fallback UI for the COMPONENTS tab changes behavior. Confirm that always rendering when selected is intentional.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

8. apps/studio/src/routes/editor/LayersPanel/index.tsx:60

• Draft comment:
  Consider whether the local 'isLocked' state should be synchronized with the engine’s observable tab state to avoid potential mismatches.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

9. apps/studio/src/routes/editor/LayersPanel/index.tsx:12

• Draft comment:
  Typographical inconsistency: The component is imported as 'ImagesTab' from './ImageTab'. It's unclear if the file name is correct or if it should be './ImagesTab' to match the naming of other pluralized components. Please double-check and rename accordingly if needed.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_wnjjziRujmBHfhjT

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 12dd2bd in 1 minute and 17 seconds

More details

• Looked at 234 lines of code in 6 files
• Skipped 0 files when reviewing.
• Skipped posting 14 drafted comments based on config settings.

1. apps/studio/src/lib/editor/engine/index.ts:184

• Draft comment:
  New getter/setter for isLayersPanelLocked looks correct. Consider adding inline documentation for clarity.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

2. apps/studio/src/routes/editor/EditPanel/StylesTab/single/FontInput.tsx:45

• Draft comment:
  When setting editorEngine.isLayersPanelLocked to true on add new font, ensure that a corresponding unlock (false) is applied when the panel is closed to avoid leaving the panel locked unintentionally.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

3. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx:66

• Draft comment:
  Closing ColorPanel sets editorEngine.brandTab to null but does not update the locked state. Verify if unlocking the layers panel explicitly is desired.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

4. apps/studio/src/routes/editor/LayersPanel/BrandTab/FontPanel/index.tsx:22

• Draft comment:
  Closing FontPanel resets editorEngine.brandTab to null. Consider if the layers panel lock should also be reset to false for consistent UX.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

5. apps/studio/src/routes/editor/LayersPanel/index.tsx:59

• Draft comment:
  The handleClick toggling of isLayersPanelLocked is concise. Double-check that the user experience is as expected when clicking on an already-selected tab to unlock the panel.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50%
  None

6. apps/studio/src/lib/editor/engine/index.ts:43

• Draft comment:
  Added isLayersPanelLocked observable; consider adding a brief comment on its intended use.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

7. apps/studio/src/routes/editor/EditPanel/StylesTab/single/FontInput.tsx:45

• Draft comment:
  Setting layersPanelTab and brandTab together; ensure documentation clarifies the distinction between these two properties.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

8. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx:66

• Draft comment:
  Switching close behavior to reset brandTab to null is clear; ensure consistent behavior across similar panels.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

9. apps/studio/src/routes/editor/LayersPanel/BrandTab/FontPanel/index.tsx:22

• Draft comment:
  Simplified close handling by directly resetting brandTab; this improves clarity and consistency.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

10. apps/studio/src/routes/editor/LayersPanel/BrandTab/index.tsx:79

• Draft comment:
  Using the brandTab property to switch between Color and Font panels; ensure that the separation from layersPanelTab is well-documented.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

11. apps/studio/src/routes/editor/LayersPanel/index.tsx:59

• Draft comment:
  Consistent use of isLayersPanelLocked to manage tab locking; consider abstracting repeated state management if similar patterns reoccur.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

12. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx:9

• Draft comment:
  Typo suggestion: The component and file are named 'BrandPalletGroup', but in the UI comments it's referred to as 'Brand Palette Groups'. Consider renaming to 'BrandPaletteGroup' for consistency.
• Reason this comment was not posted:
  Comment was on unchanged code.

13. apps/studio/src/routes/editor/LayersPanel/index.tsx:11

• Draft comment:
  The import on line 11 reads import ImagesTab from './ImageTab'; but the component is consistently used as <ImagesTab />. This inconsistency could be confusing. Please verify whether the file should be named ImagesTab (and update the import accordingly) or if the usage should be changed to <ImageTab />.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

14. apps/studio/src/routes/editor/LayersPanel/index.tsx:14

• Draft comment:
  The PagesTab import on line 14 is coming from import PagesTab from './PageTab'; while the component is referred to as PagesTab throughout the file. Please verify if there is a naming inconsistency between the file name and usage, and update the naming to be consistent.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_JiFqGUsROWoBTzFB

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.