Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advanced string off center #1024

Open
KatieWoe opened this issue Feb 27, 2025 · 2 comments
Open

Advanced string off center #1024

KatieWoe opened this issue Feb 27, 2025 · 2 comments

Comments

@KatieWoe
Copy link
Contributor

Test device
Lenovo Laptop
Operating System
Win 11
Browser
Chrome
Problem description
For phetsims/qa#1217
The "Advanced" string on the second screen in right aligned, which looks very odd.

Visuals

Image

Troubleshooting information:

!!!!! DO NOT EDIT !!!!!
Name: ‪Circuit Construction Kit: DC‬
URL: https://phet-dev.colorado.edu/html/circuit-construction-kit-dc/1.4.0-dev.1/phet/circuit-construction-kit-dc_all_phet.html
Version: 1.4.0-dev.1 2025-02-27 15:53:09 UTC
Features missing: applicationcache, applicationcache, touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/133.0.0.0 Safari/537.36
Language: en-US
Window: 1280x631
Pixel Ratio: 1.5/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

@samreid
Copy link
Member

samreid commented Mar 1, 2025

This patch fixes the problem in my testing. @matthew-blackman can you please review and double check? If it goes well, feel free to commit.

Subject: [PATCH] 

---
Index: js/view/AdvancedAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/view/AdvancedAccordionBox.ts b/js/view/AdvancedAccordionBox.ts
--- a/js/view/AdvancedAccordionBox.ts	(revision ce312f30d82655cbf6097fe20baaa7cac63fcb2e)
+++ b/js/view/AdvancedAccordionBox.ts	(date 1740798527285)
@@ -74,7 +74,7 @@
 
       // Left align the title, with no padding
       titleAlignX: 'left',
-      titleXSpacing: 0
+      titleXSpacing: 10
     } );
 
     this.mutate( options );
Index: js/view/CCKCAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/view/CCKCAccordionBox.ts b/js/view/CCKCAccordionBox.ts
--- a/js/view/CCKCAccordionBox.ts	(revision ce312f30d82655cbf6097fe20baaa7cac63fcb2e)
+++ b/js/view/CCKCAccordionBox.ts	(date 1740798748214)
@@ -9,9 +9,7 @@
 
 import BooleanProperty from '../../../axon/js/BooleanProperty.js';
 import type TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js';
-import optionize, { combineOptions } from '../../../phet-core/js/optionize.js';
-import HBox from '../../../scenery/js/layout/nodes/HBox.js';
-import HStrut from '../../../scenery/js/nodes/HStrut.js';
+import { combineOptions, EmptySelfOptions } from '../../../phet-core/js/optionize.js';
 import type Node from '../../../scenery/js/nodes/Node.js';
 import Text from '../../../scenery/js/nodes/Text.js';
 import AccordionBox, { type AccordionBoxOptions } from '../../../sun/js/AccordionBox.js';
@@ -23,9 +21,7 @@
 // constants
 const BUTTON_MARGIN = 8;
 
-type SelfOptions = {
-  strutWidth?: number;
-};
+type SelfOptions = EmptySelfOptions;
 export type CCKCAccordionBoxOptions = SelfOptions & AccordionBoxOptions;
 
 export default class CCKCAccordionBox extends AccordionBox {
@@ -38,9 +34,7 @@
    */
   public constructor( content: Node, title: TReadOnlyProperty<string>, tandem: Tandem, providedOptions?: CCKCAccordionBoxOptions ) {
 
-    const options = optionize<CCKCAccordionBoxOptions, SelfOptions, AccordionBoxOptions>()( {
-      strutWidth: 10
-    }, providedOptions );
+    const options = providedOptions;
 
     super( content, combineOptions<AccordionBoxOptions>( {
       fill: CCKCColors.panelFillProperty,
@@ -64,16 +58,11 @@
         touchAreaYDilation: BUTTON_MARGIN,
         touchAreaXDilation: BUTTON_MARGIN
       },
-      titleNode: new HBox( {
-        children: [
-          new HStrut( options.strutWidth ),
-          new Text( title, {
-            fontSize: CCKCConstants.FONT_SIZE,
-            maxWidth: 175,
-            fill: CCKCColors.textFillProperty,
-            tandem: tandem.createTandem( 'titleText' )
-          } )
-        ]
+      titleNode: new Text( title, {
+        fontSize: CCKCConstants.FONT_SIZE,
+        maxWidth: 175,
+        fill: CCKCColors.textFillProperty,
+        tandem: tandem.createTandem( 'titleText' )
       } ),
       tandem: tandem
     }, options ) );

@samreid samreid removed their assignment Mar 1, 2025
matthew-blackman added a commit that referenced this issue Mar 1, 2025
@matthew-blackman
Copy link
Contributor

Code changes look great and the title text has pixel-perfect consistency with production. Thanks @samreid! Marking as ready-for-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants