forked from theRAPTLab/meme
-
Notifications
You must be signed in to change notification settings - Fork 0
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
dev-tv/fix-offscreen-entity: Fixes #7 caused by UpdateDimensions() wrong height #17
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
villagrat
changed the title
dev-tv/fix-offscreen-entity: Fixes #7 caused by UpdateDimensions() wrong height
DRAFT: dev-tv/fix-offscreen-entity: Fixes #7 caused by UpdateDimensions() wrong height
Jun 11, 2024
villagrat
force-pushed
the
dev-tv/fix-offscreen-entity
branch
from
June 11, 2024 21:01
754fb3c
to
d66e014
Compare
note: eslint is used for "live linting" only in the editor - remove 'no-unescaped-entities' rule because it's an unnecessary guard for experienced dev - change ecmaVersion from 2018 to 2020 to support optional chaining changes to MEMEstyles - remove "prettier/react" eslint plugin rule extension to remove warning; prettier/react is now part of prettier eslint plugin.
FIX: Build System Configuration Errors
Sakelun
approved these changes
Jun 18, 2024
Sakelun
changed the title
DRAFT: dev-tv/fix-offscreen-entity: Fixes #7 caused by UpdateDimensions() wrong height
dev-tv/fix-offscreen-entity: Fixes #7 caused by UpdateDimensions() wrong height
Jun 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes #7 where entities and models were being displayed offscreen when created. The issue was identified in the
ViewMain.jsx
file and has been addressed by modifying the calculation ofviewHeight
. Also we applied thestyled
util to a container<div />
element onViewMain
to avoid new entities/outcomes be covered by the sidebarUser-visible changes:
Entities and models are now correctly displayed within the screen boundaries when created.
Developer-relevant changes:
The calculation of
viewHeight
inViewMain.jsx
has been modified.Testing Steps
Technical Notes
Changes to
viewHeight
calculation:The line
viewHeight: Math.min(viewHeight, innerHeight)
was usingMath.min
to calculate theviewHeight
. This was causing the container height to be set to 0, putting newly created entities offscreen when eitherviewHeight
orinnerHeight
was 0. The calculation has been changed to use Math.max instead, ensuring thatviewHeight
is the larger of the two values.