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

Refactor WorkflowCard #334

Merged

Conversation

joebochill
Copy link
Collaborator

@joebochill joebochill commented Feb 2, 2024

Changes proposed in this Pull Request:

  • Remove extra view wrapper (use ImageBackground as the root element)
  • Refactor useScreenWidth -> useScreenDimensions
    • use the existing useDeviceDimensions hook for width/height instead of writing our own logic
    • return the screen dimensions and the isTablet utility boolean
  • Add a mock WorkflowCardHeader for testing (instead of trying to hack based on WorkflowCardBody)
  • Remove Header component from example page — Header component will not / should not be used with the WorkflowCard component — that's why we have a WorkflowCardHeader.
    • this was causing confusion and obscuring the behavior of the card by being in the way at the top of the screen
  • moved mobile and tablet styles out into 'classes' to eliminate multiple inline conditional logic statements
  • Removed recursive check for WorkflowCardHeader (expected usage will have it in the direct children of WorkflowCard only)
  • Update background image behavior to tile and set the background color on the ImageBackground
    • if we want this to be customizable, we can add a prop for it, or users can simply add a style rule on the WorkflowCard (this is probably sufficient)

Screenshots

To Test:

  • yarn start:example
  • Click on the button to view the example

Any specific feedback you are looking for?

  • I didn't specifically test this on a tablet device (I was just verifying styles applied as expected on landscape phone), so it would be good to sanity check on a device where the card actually floats in the middle.

@github-actions github-actions bot added the brightlayer-ui Used to identify Brightlayer UI platform issues for easy filtering label Feb 2, 2024
@manojleaton
Copy link
Contributor

Tested the app in both android and ios tablets, Please check the screenshots attached
Screenshot 2024-02-05 at 1 24 20 PM
Screenshot 2024-02-05 at 1 15 32 PM

@manojleaton manojleaton merged commit 553176f into feature/5097-Implement-WorkflowCard-component Feb 5, 2024
3 checks passed
@manojleaton manojleaton deleted the fix/refactor-card branch February 5, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brightlayer-ui Used to identify Brightlayer UI platform issues for easy filtering
Development

Successfully merging this pull request may close these issues.

None yet

2 participants