You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "richardfogaca (via GitHub)" <gi...@apache.org> on 2025/10/30 21:42:53 UTC

[PR] fix(sqllab): align refresh buttons with select input fields [superset]

richardfogaca opened a new pull request, #35917:
URL: https://github.com/apache/superset/pull/35917

   ### SUMMARY
   
   This PR fixes the vertical alignment issue of refresh buttons next to the Schema and Table selector fields in SQL Lab and the Add Dataset interface.
   
   **Problem:**
   The refresh icons were vertically misaligned with their corresponding select input fields due to two issues:
   1. A hardcoded `margin-top: 20px` was applied to the refresh button container
   2. More fundamentally, the refresh buttons were aligned to the entire Select/AsyncSelect component (which includes the label + input field), causing them to align vertically centered against both elements rather than just the input field
   
   **Root Cause:**
   When using the `header` prop on Select/AsyncSelect components, the label becomes part of the component's internal structure. This caused the flex container holding the refresh button to align against the total height of the label + input, positioning the button between the label and input rather than horizontally aligned with just the input field.
   
   **Solution:**
   1. Removed the `margin-top: 20px` from the `.refresh` CSS class in both `DatabaseSelector` and `TableSelector` components
   2. Extracted labels from the Select/AsyncSelect components by:
      - Removing the `header` prop from Select/AsyncSelect components
      - Creating a separate `StyledFormLabel` component positioned outside the select component
   3. This allows refresh buttons to align horizontally with the input field only, achieving proper vertical alignment
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   **BEFORE:**
   <img width="418" height="422" alt="sqllab refresh" src="https://github.com/user-attachments/assets/60663790-7ab9-4a0a-94eb-5ff89290a8f3" />
   
   **AFTER:**
   <img width="388" height="226" alt="Screenshot 2025-10-30 at 17 54 37" src="https://github.com/user-attachments/assets/6702c694-1875-4de2-a2e5-488af4aa46fa" />
   <img width="392" height="70" alt="Screenshot 2025-10-30 at 18 41 54" src="https://github.com/user-attachments/assets/d8e518e1-fe0c-4801-bfd4-666bd089f6f6" />
   
   
   ### TESTING INSTRUCTIONS
   
   **Test in SQL Lab:**
   1. Navigate to **SQL Lab** → **SQL Editor**
   2. Observe the **Schema** selector field
   3. Verify the refresh icon is horizontally aligned with the schema select input (not the label)
   4. Observe the **See table schema** selector field
   5. Verify the refresh icon is horizontally aligned with the table select input (not the label)
   6. Check that spacing to the left of refresh buttons is consistent (4px)
   
   **Test in Add Dataset:**
   1. Navigate to **Data** → **Datasets**
   2. Click **+ Dataset** button
   3. In the left panel, observe the **Schema** selector
   4. Verify the refresh icon is horizontally aligned with the schema select input
   5. Observe the **Table** selector (note: shows "Table" label instead of "See table schema" when `sqlLabMode={false}`)
   6. Verify the refresh icon is horizontally aligned with the table select input
   
   ### ADDITIONAL INFORMATION
   - [x] Changes UI
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(sqllab): align refresh buttons with select input fields [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #35917:
URL: https://github.com/apache/superset/pull/35917#discussion_r2481300442


##########
superset-frontend/src/components/TableSelector/index.tsx:
##########
@@ -78,6 +77,12 @@ const TableSelectorWrapper = styled.div`
   `}
 `;
 
+const StyledFormLabel = styled(FormLabel)`
+  display: block;
+  font-size: ${({ theme }) => theme.fontSizeSM}px;
+  margin-bottom: ${({ theme }) => theme.sizeUnit}px;
+`;

Review Comment:
   Should we extract this?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(sqllab): align refresh buttons with select input fields [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #35917:
URL: https://github.com/apache/superset/pull/35917


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(sqllab): align refresh buttons with select input fields [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #35917:
URL: https://github.com/apache/superset/pull/35917#issuecomment-3470389784

   <details open>
     <summary><strong>Interaction Diagram by <a href="https://bito.ai#sequence_diagram">Bito</a></strong></summary>
   
   ```mermaid
   sequenceDiagram
   participant UI as UI Layer<br/>(DatasourceEditor)
   participant DS as DatabaseSelector<br/>🔄 Updated | ●●○ Medium
   participant TS as TableSelector<br/>🔄 Updated | ●●○ Medium
   participant SFL as StyledFormLabel<br/>🟩 Added | ●●● High
   participant RSR as renderSelectRow
   participant Select as Select Component
   participant Theme as Theme Provider
   Note over SFL: New styled component<br/>extracted from Select header prop
   UI->>DS: Render DatabaseSelector
   DS->>SFL: Create styled label
   SFL->>Theme: Apply theme styling
   DS->>RSR: Render catalog/schema select
   RSR->>Select: Render Select without header
   Select-->>RSR: Return rendered select
   UI->>TS: Render TableSelector
   TS->>SFL: Create styled label
   TS->>RSR: Render table select
   RSR->>Select: Render Select without header
   ```
   Critical path: UI Layer-&gt;DatabaseSelector-&gt;StyledFormLabel-&gt;Theme Provider; TableSelector-&gt;StyledFormLabel-&gt;renderSelectRow-&gt;Select Component
   
   
   > **Note:** Labels are refactored from Select component&#39;s header prop to a separate StyledFormLabel component rendered before renderSelectRow(). This improves styling consistency and label positioning by centralizing label rendering logic. Upstream components (DatasourceEditor, SqlEditorLeftBar) are unaffected.
   
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(sqllab): align refresh buttons with select input fields [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #35917:
URL: https://github.com/apache/superset/pull/35917#issuecomment-3470381305

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org