-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feature/bl UI 3279 mobile view tabs #805
Conversation
Co-Authored-By: Joseph Boyle <boyle.p.joseph@gmail.com> Co-Authored-By: Thomas Dailey <daileytj89@gmail.com>
Co-Authored-By: Thomas Dailey <daileytj89@gmail.com>
Co-Authored-By: Thomas Dailey <daileytj89@gmail.com> Co-Authored-By: Joseph Boyle <boyle.p.joseph@gmail.com>
Co-Authored-By: Joseph Boyle <boyle.p.joseph@gmail.com>
Co-Authored-By: Thomas Dailey <daileytj89@gmail.com>
Co-Authored-By: Thomas Dailey <daileytj89@gmail.com>
Co-Authored-By: Thomas Dailey <daileytj89@gmail.com>
Co-Authored-By: Thomas Dailey <daileytj89@gmail.com>
Co-Authored-By: Thomas Dailey <daileytj89@gmail.com> Co-Authored-By: huayunh <8997218+huayunh@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrupu Prettier and lint are failing.
Please run yarn precommit.
@andrupu , can you add figma link ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please resolve your lint error about "Missing return type on function"
- Please run prettier
- Please add the black overlay thing (scrim) we mentioned. Ideally, onclick the black overlay should dismay the expanded bottom sheet
- Please add the animation to expand / collapse the drawer
- Please remove all the unused code that you commented out
- Something is causing an undesired overflow. For example, this should not be scrolling up and down like this:
https://github.com/etn-ccis/blui-react-component-library/assets/8997218/acf7693e-61b9-443e-a0e1-0610b37a4dae
@@ -21,7 +26,7 @@ const PreviewComponentWithCode: React.FC<PreviewComponentProps> = (props): JSX.E | |||
* We take the height of the appbar/tabs out of the 70vh portion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to update the inline comment
@@ -253,30 +274,135 @@ const PlaygroundDrawer = (props: DrawerProps): JSX.Element => { | |||
); | |||
}; | |||
|
|||
type PlaygroundTabPanelProps = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we usually take the type declaration out of the component scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, both PlaygroundTabPanelProps
and PlaygroundTabPanel
can probably be taken out of PlaygroundDrawer
to make PlaygroundDrawer
code shorter and easier to read
transform: 'none !important', | ||
visibility: 'visible !important', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not be turning the drawer on/off via these CSS attributes
Visit the preview URL for this PR (updated for commit 8035fa9): https://blui-react-docs--pr805-feature-blui-3279-mo-poewji02.web.app (expires Sun, 01 Oct 2023 19:38:43 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 34d39fa5aab0ea0cf95074e8e76f68829e7a8c65 |
Closing due to inactivity. It's going to be a while before this refactoring work is ready to go into dev...I would suggest setting up a new feature branch to use as the target for the incremental work until it is ready to be merged into dev. |
Fixes # BLUI-3279
Changes proposed in this Pull Request:
-Update dev docs styling to match the designs for mobile view on smaller screens.
-Main point of address will be updates to the Playground screen on AppBar Example.
-This will NOT include multilayer drawer updates.
To Test: