Skip to content

fix: change tab block-size to min-block-size to allow height to grow - #1219

Merged
maxelkins merged 8 commits into
mainfrom
preview-in-new-tab-causing-wrapping
Jun 18, 2025
Merged

fix: change tab block-size to min-block-size to allow height to grow#1219
maxelkins merged 8 commits into
mainfrom
preview-in-new-tab-causing-wrapping

Conversation

@maxelkins

@maxelkins maxelkins commented Jun 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes: https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/690

Summary

Issue related:

  • Change tab block-size to min-block-size to allow height to grow
  • Also change to correct custom property to ensure minimum target size

Fixing failing tests:

  • The Skulpt runner implementation was hiding tab/elements by setting block-size (height) to 0. As a result when using min-block-size this was causing cypress visiblity test to fail.
  • It was previously passing since cypress considers height 0 as not visible.
  • This implementation has now been changed to set display: none to properly remove the element
  • There are 2 other potential changes that could be made here
    1. Conditially render the elements instead - as the PyodideRunner does
    2. Change the cypress test to check for existence instead
  • I've opted not to change the above to keep this PR smaller

Demo

Screenshot 2025-06-17 at 16 32 17

@maxelkins
fix: change block-size to min-block-size to allow height to grow
361aba2 
also change to correct custom property to ensure minimum target size
@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 17, 2025 15:40 — with GitHub Actions Inactive
@maxelkins maxelkins changed the title fix: change block-size to min-block-size to allow height to grow fix: change tab block-size to min-block-size to allow height to grow Jun 17, 2025
@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 17, 2025 16:00 — with GitHub Actions Inactive
danhalson
danhalson reviewed Jun 17, 2025
View reviewed changes
Comment thread CHANGELOG.md Outdated
@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 17, 2025 16:30 — with GitHub Actions Inactive
@maxelkins

maxelkins commented Jun 17, 2025

Copy link
Copy Markdown
Contributor Author

@danhalson fixed the changelog issue, there was another unreleased change that was there too

danhalson
danhalson approved these changes Jun 18, 2025
View reviewed changes

@danhalson danhalson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 18, 2025 09:02 — with GitHub Actions Inactive
@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 18, 2025 12:44 — with GitHub Actions Inactive
@maxelkins
fix: change hiding implmentation to use display rather than height/bl…
65f039c 
…ock-size

- Switched from manipulating `block-size` to `display: none` for more robust and explicit hiding of the visual output panel and its corresponding tab.
- Ensures elements are truly removed from the layout, preventing subtle rendering issues and improving reliability for Cypress visibility assertions
@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 18, 2025 15:08 — with GitHub Actions Inactive
@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 18, 2025 15:14 — with GitHub Actions Inactive
@maxelkins
maxelkins requested a review from danhalson June 18, 2025 15:19
danhalson
danhalson approved these changes Jun 18, 2025
View reviewed changes

@danhalson danhalson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@maxelkins
maxelkins temporarily deployed to previews/1219/merge June 18, 2025 15:38 — with GitHub Actions Inactive
@maxelkins
maxelkins enabled auto-merge (squash) June 18, 2025 15:39
@maxelkins
maxelkins merged commit b9481df into main Jun 18, 2025
8 checks passed
@maxelkins
maxelkins deleted the preview-in-new-tab-causing-wrapping branch June 18, 2025 15:41
@jamiebenstead jamiebenstead mentioned this pull request Jul 22, 2025
Merged
jamiebenstead added a commit that referenced this pull request Jul 22, 2025
@jamiebenstead
V0.30.2 (#1235)
757831a 
## [0.30.2] - 2025-07-22

### Fixed

- Styling issue preventing scrolling in the sidebar (#1216)
- Styling issue on status bar on mobile (#1217)
- Change tab `block-size` to `min-block-size` to allow for height
increase when text wraps (#1219)
- Changed `SkulptRunner.jsx` implementation of hiding elements to use
`display: none` rather than `block-size: 0` (#1219)
- Enabled `hyphens: auto` globally (with exceptions) to prevent overflow
with longer words (#1215)
- Removed fixed size from `ProjectBar` to prevent overflow when text
wraps (#1221)
- Added missing translation strings (#1222)
- Changed `colour_raw` to now correctly return a 4-tuple (R, G, B,
Clear) in simulation

### Changed

- Improved status bar styling (#1221)
- Added horizontal scrollbar to editor (#1228)
- Add theme for scrollbars (#1233)
jamiebenstead added a commit that referenced this pull request Jul 28, 2025
@jamiebenstead
V0.30.2 (#1235)
abafa50 
## [0.30.2] - 2025-07-22

### Fixed

- Styling issue preventing scrolling in the sidebar (#1216)
- Styling issue on status bar on mobile (#1217)
- Change tab `block-size` to `min-block-size` to allow for height
increase when text wraps (#1219)
- Changed `SkulptRunner.jsx` implementation of hiding elements to use
`display: none` rather than `block-size: 0` (#1219)
- Enabled `hyphens: auto` globally (with exceptions) to prevent overflow
with longer words (#1215)
- Removed fixed size from `ProjectBar` to prevent overflow when text
wraps (#1221)
- Added missing translation strings (#1222)
- Changed `colour_raw` to now correctly return a 4-tuple (R, G, B,
Clear) in simulation

### Changed

- Improved status bar styling (#1221)
- Added horizontal scrollbar to editor (#1228)
- Add theme for scrollbars (#1233)
@patch0 patch0 mentioned this pull request Oct 3, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danhalson danhalson danhalson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@maxelkins @danhalson