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

Tweaks to Tab UI #164

Merged
merged 3 commits into from
Jul 29, 2024
Merged

Tweaks to Tab UI #164

merged 3 commits into from
Jul 29, 2024

Conversation

WardBrian
Copy link
Collaborator

  • Makes the tab bars scrollable if necessary
  • Switched out the splitter element so it places nicer
    • The one we were using before relies on computed styles, which don't exist if the element or its parent is hidden. As such, the splitter was un-mounting its children if it was hidden.
    • The easiest way to observe this was to run one of the scripts then change the tab. The script would still run, updating the data etc., but the output area would stay blank when you tabbed back over because it was unmounted.

I preferred the look of the other splitter, and this one seems to be a bit heavier in terms of dependencies, so I'm open to alternatives here.

@WardBrian WardBrian requested a review from jsoules July 29, 2024 14:19
@jsoules
Copy link
Collaborator

jsoules commented Jul 29, 2024

Not super fussed about the code changes, but I'm noting that the main.stan window refreshes when showing/hiding the errors pane, which doesn't happen on the main branch.

@WardBrian
Copy link
Collaborator Author

Yeah, I noticed that. This version of a splitter doesn't support a variable number of children -- which is good, since we were running into issues with the size persisting from the old splitter because it was re-rendering based on the children array, but it does mean to get the old behavior we need to return a different component based on the value of syntaxWindowVisible. Do you know of any way to avoid re-rendering in that case?

@jsoules
Copy link
Collaborator

jsoules commented Jul 29, 2024

Hmm... One could memoize the main.stan pane maybe? I'm not sure if that would actually help, though, since its parent would be conditionally popping in and out, which is probably going to cause a rerender regardless...

@jsoules
Copy link
Collaborator

jsoules commented Jul 29, 2024

Hmm... One could memoize the main.stan pane maybe?

Yeah, that doesn't help, since the parent component is changing anyway. Maybe keep the Split component in and vary the hidden status of the error window? Let me try that.

@jsoules
Copy link
Collaborator

jsoules commented Jul 29, 2024

Maybe keep the Split component in and vary the hidden status of the error window? Let me try that.

Nope, that isn't it either--the Split component is inserting a couple additional layers of divs that would need to be hidden themselves.

@WardBrian
Copy link
Collaborator Author

@jsoules I found another option I could set to make the actual splitter itself invisible, so the fact that it's always two children no longer matters

@jsoules
Copy link
Collaborator

jsoules commented Jul 29, 2024

@jsoules I found another option I could set to make the actual splitter itself invisible, so the fact that it's always two children no longer matters

Oh awesome!

@WardBrian WardBrian merged commit 784e70c into main Jul 29, 2024
2 checks passed
@WardBrian WardBrian deleted the tab-tweaks branch July 29, 2024 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants