justinpark commented on code in PR #34629:
URL: https://github.com/apache/superset/pull/34629#discussion_r2267979811


##########
superset-frontend/packages/superset-ui-core/src/components/ListViewCard/index.tsx:
##########
@@ -94,8 +100,6 @@ const TitleLink = styled.span`
 
 const TitleRight = styled.span`
   ${({ theme }) => css`
-    position: absolute;
-    right: -1px;

Review Comment:
   Please change `right` to `inset-inline-end` instead of removing this 
absolute position.



##########
superset-frontend/packages/superset-ui-core/src/components/ListViewCard/index.tsx:
##########
@@ -242,14 +246,18 @@ function ListViewCard({
                     {title}
                   </TitleLink>
                 </Tooltip>
-                {titleRight && <TitleRight>{titleRight}</TitleRight>}
                 <div className="card-actions" data-test="card-actions">
                   {actions}
                 </div>
               </div>
             </TitleContainer>
           }
-          description={description}
+          description={
+            <div className="card-description">
+              {description}
+              {titleRight && <TitleRight>{titleRight}</TitleRight>}

Review Comment:
   After making the CSS modifications mentioned 
[above](https://github.com/apache/superset/pull/34629/files#diff-04159a06adacf9c3eb56a99b1aee4817f6033abac3de65e8b95e29b2e68b59ecL98),
 I am confident that we can return the titleRight component to its original 
position.
   
   ```suggestion
   ```



##########
superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx:
##########
@@ -219,6 +232,14 @@ export class Theme {
     this.setConfig(newConfig);
   }
 
+  setDirection(direction: DirectionType): void {
+    const newTheme = { ...this.theme };
+    newTheme.direction = direction;
+
+    // Update the providers with the fully formed theme
+    this.updateProviders(newTheme, this.antdConfig, this.createCache());

Review Comment:
   nit: You can skip the `newTheme` assignment.
   ```suggestion
       // Update the providers with the fully formed theme
       this.updateProviders({ ...this.theme, direction }, this.antdConfig, 
this.createCache());
   ```



##########
superset-frontend/packages/superset-ui-core/src/components/DynamicEditableTitle/index.tsx:
##########
@@ -56,9 +56,9 @@ const titleStyles = (theme: SupersetTheme) => css`
 
   & .input-sizer {
     position: absolute;
-    left: -9999px;
     display: inline-block;
     white-space: pre;
+    opacity: 0;

Review Comment:
    I don’t think this change is related to RTL support.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to