Update Night Sky Panel's Markings tab + Small Sun Tab refactor by WeirdRubberDuck · Pull Request #207 · OpenSpace/OpenSpace-WebGui

WeirdRubberDuck · 2026-03-20T08:40:02Z

Closes OpenSpace/OpenSpace#3753 by adding a new ToggleCard component and improving the layout in the Night Sky Panel's Markings tab. IN discussion with @ylvaselling, I also changed the design for the Cardinal directions part to better reflect how this control works, and that it's different from the other toggles.

Drafting as the following needs to be fixed before merging:

Should be merged after Feature/property tree topic #195 and adapted/tested with the updated visibility code
The "Hide all" button for each Markings group needs action implementation by @micahnyc

New VS old Markings tab:

New VS old Sun tab:

… texture

…rest of UI

WeirdRubberDuck · 2026-03-20T08:42:36Z

The layout is now more responsive and uses an adaptive gridding similar to the skybrowser panel. Demonstration:

2026-03-20.09-43-50.mov

Co-authored-by: Alexander Bock <mail@alexanderbock.eu>

# Conflicts: # src/panels/NightSkyPanel/tabs/MarkingsTab/SceneGraphNodeToggle.tsx

# Conflicts: # src/panels/NightSkyPanel/tabs/MarkingsTab/CardinalDirectionsBoxes.tsx # src/panels/NightSkyPanel/tabs/MarkingsTab/ConstellationsArtToggle.tsx # src/panels/NightSkyPanel/tabs/MarkingsTab/ConstellationsLabelsToggle.tsx # src/panels/NightSkyPanel/tabs/MarkingsTab/ConstellationsLinesToggle.tsx # src/panels/NightSkyPanel/tabs/MarkingsTab/MarkingBox.tsx

micahnyc · 2026-04-15T01:23:52Z

Actions now available here: https://github.com/OpenSpace/OpenSpace-WebGui/pull/207/changes and I updated this branch to call them.

…com/OpenSpace/OpenSpace-WebGui into issue/3753-nightsky-markings-layout

WeirdRubberDuck · 2026-04-15T09:34:33Z

The "Cardinal Directions" part's design was changed once again... Now it corresponds more to how it actually works - with the scene graph node being enabled/disabled and then which texture is set. It also allows setting the visuals before showing the direction.

This is how it looks now:

micahnyc · 2026-04-16T17:20:49Z

Looks and works great! Tested all the changes, and I updated the actions in the main repo to fix the hide all stuff

engbergandreas

Sorry for all the onClick functions, but might as well get through them. We should probably make a separate pass through on this so that we're up-to-date with our readme. I only looked at the code and nothing sticks out. Looks way nicer now, great job!

engbergandreas · 2026-04-17T06:59:02Z

+        "add-trail-simulation-date": "Add trail for Simulation Date",
+        "add-trail-today": "Add trail for Today",


Suggested change

"add-trail-simulation-date": "Add trail for Simulation Date",

"add-trail-today": "Add trail for Today",

"add-trail-simulation-date": "Add trail for simulation date",

"add-trail-today": "Add trail for today",

?

Suggested change

"add-trail-simulation-date": "Add trail for Simulation Date",

"add-trail-today": "Add trail for Today",

"add-trail-simulation-date": "Add trail for 'Simulation Date'",

"add-trail-today": "Add trail for 'Today'",

Alternative suggestion to make it more clear that that is the important part of the sentence

engbergandreas · 2026-04-17T06:59:23Z

-        "show-stars": "Show Stars",
+        "show-stars": "Show stars",
        "show-during-day": {
          "label": "Show during Day",


Suggested change

"label": "Show during Day",

"label": "Show during day",

engbergandreas · 2026-04-17T07:05:01Z

+          onChange={(value: string) => {
+            switch (value) {
+              case 'small':
+                luaApi?.action.triggerAction(Data['small'].showAction);
+                if (isTextureOn('marks')) {
+                  // The action for showing the small letters also hides the marks, so we
+                  // need to show them again if they were enabled before
+                  luaApi?.action.triggerAction(Data['marks'].showAction);
+                }
+                setSizeToggle('small');
+                break;
+              case 'large':
+                luaApi?.action.triggerAction(Data['large'].showAction);
+                if (isTextureOn('marks')) {
+                  // The action for showing the large letters also hides the marks, so we
+                  // need to show them again if they were enabled before
+                  luaApi?.action.triggerAction(Data['marks'].showAction);
+                }
+                setSizeToggle('large');
+                break;
+              default:
+                setSizeToggle(undefined);
+                break;
+            }
+          }}


Should this be its own function instead?

engbergandreas · 2026-04-17T07:09:11Z

+          onChange={(event) =>
+            luaApi?.action.triggerAction(
+              event.target.checked ? Data['marks'].showAction : Data['marks'].hideAction
+            )


Should be wrapped with in a function body right?

engbergandreas · 2026-04-17T07:10:44Z

-        />
-      }
+    <MarkingsToggleLayout
+      onClick={() => checkboxChange(!isVisible)}


Suggested change

onClick={() => checkboxChange(!isVisible)}

onClick={() => { checkboxChange(!isVisible)} }

To follow our new guidelines?

This might need some discussion, at least in my mind. I sent out a question about onclicks specifically in Slack a while ago, but never got a reply

This suggestion is missing a semicolon, which is required when including brackets, and due to our linting, this would result in three lines instead of one:

Suggested change

onClick={() => checkboxChange(!isVisible)}

onClick={() => {

checkboxChange(!isVisible);

}}

Personally, I feel like it adds clutter in a way that I find undesired for simple onclick functions, and I'm wondering if we want ot make an exception for things like onChange and onClick. In these cases, it's already quite obvious that there is no value being returned. Thoughts?

I know I'm also biased towards writing things in one line generally, so would like to hear what you think @ylvaselling @engbergandreas and maybe @alexanderbock :)

Pinged you all in Slack as well

engbergandreas · 2026-04-17T07:14:26Z

+          variant={'subtle'}
+          onClick={() =>
+            luaApi?.action.triggerAction('os.nightsky.HideAllConstellationElements')
+          }


engbergandreas · 2026-04-17T07:16:53Z

@@ -83,20 +86,22 @@ export function SunTab() {
          <Button onClick={() => setAngularSize(0.8)}>


theese 3 aswell - sorry

engbergandreas · 2026-04-17T07:17:00Z

-          </ActionIcon>
+          <ActionIcon.Group>
+            <ActionIcon
+              onClick={() => setAngularSize(angularSize - 0.1)}


engbergandreas · 2026-04-17T07:17:09Z

+              <MinusIcon />
+            </ActionIcon>
+            <ActionIcon
+              onClick={() => setAngularSize(angularSize + 0.1)}


engbergandreas · 2026-04-17T07:17:17Z

+        <Button
+          size={'compact-sm'}
+          leftSection={<EyeOffIcon />}
+          onClick={() => luaApi.fadeOut(`{${sunTrailTag}}`)}


alexanderbock · 2026-04-17T20:45:48Z

      "aria-labels": {
-        "small": "Show cardinal directions (small)",
-        "large": "Show cardianl directions (large)",
+        "toggle": "Toggle cardinal directions' size",


Suggested change

"toggle": "Toggle cardinal directions' size",

"toggle": "Toggle cardinal directions' sizes",

alexanderbock · 2026-04-17T20:46:51Z

+        "add-trail-simulation-date": "Add trail for Simulation Date",
+        "add-trail-today": "Add trail for Today",


Suggested change

"add-trail-simulation-date": "Add trail for Simulation Date",

"add-trail-today": "Add trail for Today",

"add-trail-simulation-date": "Add trail for 'Simulation Date'",

"add-trail-today": "Add trail for 'Today'",

Alternative suggestion to make it more clear that that is the important part of the sentence

alexanderbock · 2026-04-17T20:48:59Z

+  /**
+   * The identifier of the scene graph node that this toggle represents
+   */


Suggested change

/**

* The identifier of the scene graph node that this toggle represents

*/

// The identifier of the scene graph node that this toggle represents

?

WeirdRubberDuck added 10 commits March 19, 2026 11:33

Update layouting in Night sky panel markings tab

99993ac

Tighten up a bit

ac625bb

Redesign cardinal directions part to look different than the others

cf03255

Redesign layout for markings tab and introduce a new toggle component

fc58b88

Refactor cardinal direction and make it listen to the currently shown…

1ef5591

… texture

Add animation to ToggleCard and move to general components

cf16098

Rename components to better reflect the new toggle component

39f0b53

Fix aria label for scene graph node toggle

6d193db

Add placeholder buttons for hide all per group

47b5b30

Small cleanup of Sun tab and make text labels better match format in …

da83af6

…rest of UI

WeirdRubberDuck marked this pull request as draft March 20, 2026 08:40

Small updates

4e9eabd

Only pad in y direction to better fit text

68bdb17

WeirdRubberDuck requested a review from micahnyc March 20, 2026 08:45

alexanderbock approved these changes Mar 26, 2026

View reviewed changes

Comment thread src/components/ToggleCard/ToggleCard.module.css Outdated

Comment thread src/panels/NightSkyPanel/tabs/MarkingsTab/SceneGraphNodeToggle.tsx Outdated

WeirdRubberDuck and others added 3 commits March 27, 2026 09:13

Apply suggestions from code review

f9f94b8

Co-authored-by: Alexander Bock <mail@alexanderbock.eu>

Small refactor

fab388d

# Conflicts: # src/panels/NightSkyPanel/tabs/MarkingsTab/SceneGraphNodeToggle.tsx

micahnyc mentioned this pull request Apr 15, 2026

Add hide actions and tags for night sky elements OpenSpace/OpenSpace#3998

Merged

added functions calls now available in corresponding openspace branch

713b70b

WeirdRubberDuck and others added 4 commits April 15, 2026 10:05

Merge branch 'issue/3753-nightsky-markings-layout' of https://github.…

d643bb4

…com/OpenSpace/OpenSpace-WebGui into issue/3753-nightsky-markings-layout

Merge branch 'master' into issue/3753-nightsky-markings-layout

ab53467

Fix the cardinal directions by redesigning it, again

601ddb8

ci: auto-fix prettier & eslint issues

b33137d

WeirdRubberDuck marked this pull request as ready for review April 15, 2026 09:32

WeirdRubberDuck added 2 commits April 15, 2026 11:41

Add translation and small refactor of function names

06a1975

Move icons to icons file

eff704e

micahnyc approved these changes Apr 16, 2026

View reviewed changes

engbergandreas approved these changes Apr 17, 2026

View reviewed changes

alexanderbock approved these changes Apr 17, 2026

View reviewed changes

		"add-trail-simulation-date": "Add trail for Simulation Date",
		"add-trail-today": "Add trail for Today",

	onClick={() => checkboxChange(!isVisible)}
	onClick={() => { checkboxChange(!isVisible)} }

-      onClick={() => checkboxChange(!isVisible)}
+      onClick={() => {
+        checkboxChange(!isVisible);
+      }}

		@@ -83,20 +86,22 @@ export function SunTab() {
		<Button onClick={() => setAngularSize(0.8)}>

	"toggle": "Toggle cardinal directions' size",
	"toggle": "Toggle cardinal directions' sizes",

Conversation

WeirdRubberDuck commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WeirdRubberDuck commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

micahnyc commented Apr 15, 2026

Uh oh!

WeirdRubberDuck commented Apr 15, 2026

Uh oh!

micahnyc commented Apr 16, 2026

Uh oh!

engbergandreas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WeirdRubberDuck Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

WeirdRubberDuck commented Mar 20, 2026 •

edited

Loading

WeirdRubberDuck commented Mar 20, 2026 •

edited

Loading

WeirdRubberDuck Apr 17, 2026 •

edited

Loading