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

Item props refresh #155

Merged
merged 15 commits into from Mar 17, 2022
Merged

Conversation

pablo-mayrgundter
Copy link
Member

@pablo-mayrgundter pablo-mayrgundter commented Mar 16, 2022

ItemProperties changes:

  • Use header/paragraph for props instead of table. This makes better use of vertical space.
  • Densify spacing.
  • Use MobileDrawer for swipeable 3-stage drawer on mobile. NOTE: Please test thoroughly for swipe behavior on iOS different browsers. I saw lots of warnings about incompatible cross-device behaviors.

Partially addresses the content visibility in #148. Properties are now header+text instead of table. Also mobile has full-screen toggle.

esbuild:

  • keepNames = true For minification to avoid corrupting IFC classnames and display in props panel, but also almost back to original bundle size. Now at 2.5MB

@pablo-mayrgundter pablo-mayrgundter added the bug Something isn't working label Mar 16, 2022
@OlegMoshkovich
Copy link
Member

image

Copy link
Member

@OlegMoshkovich OlegMoshkovich left a comment

Choose a reason for hiding this comment

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

Checked on firefox and safari mobile both look great.
Changing the icon to 'x' when the drawer is open would look great.

src/Components/ItemProperties.jsx Outdated Show resolved Hide resolved
src/Components/ItemProperties.jsx Outdated Show resolved Hide resolved
},
'& h2': {
fontSize: '1.1em',
fontWeight: 800,
Copy link
Member

Choose a reason for hiding this comment

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

this seems to much - keep it at 600 - maybe.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, gave it a try but doesn't go between.. it's either thin or heavy, at least for me... give this a try.

Copy link
Member

Choose a reason for hiding this comment

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

looks good!

src/Components/ItemPropertiesDrawer.jsx Show resolved Hide resolved
swipeAreaWidth={drawerBleeding}
disableSwipeToOpen={false}>
<StyledBox className={classes.contentContainer}>
<Puller onClick={toggleDrawer}/>
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

My net can't take figma right now :( I think this is as discussed tho... lmk!

Copy link
Member

Choose a reason for hiding this comment

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

cool!

src/Components/MobileDrawer.jsx Show resolved Hide resolved
src/Components/OperationsGroup.jsx Show resolved Hide resolved
@pablo-mayrgundter
Copy link
Member Author

PTAL

@OlegMoshkovich OlegMoshkovich merged commit 2ddbf31 into bldrs-ai:main Mar 17, 2022
@pablo-mayrgundter pablo-mayrgundter deleted the item-props-refresh branch April 22, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

None yet

2 participants