Skip to content

feat: drop Text component in favour of Text component - #1663

Merged
davehakkens merged 1 commit into
masterfrom
chore/move-Text-to-components-lib
May 17, 2022
Merged

feat: drop Text component in favour of Text component#1663
davehakkens merged 1 commit into
masterfrom
chore/move-Text-to-components-lib

Conversation

@thisislawatts

@thisislawatts thisislawatts commented May 14, 2022

Copy link
Copy Markdown
Contributor

PR Checklist

PR Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Developer experience (improves developer workflows for contributing to the project)

Description

Removes our own Text component in favour of directly using the Text component from theme-ui. Our implementation was a thin wrapper that introduced a few attributes like txtCenter which seems like it would be better to set via the sx attribute.

This introduces changes like the following:

// Before
<Text txtCenter>I am text</Text>

// After
<Text sx={{textAlign: 'center'}}>I am text</Text>

There are no user facing changes here, the intention here is to simplify the experience when building UI components. Before there were two possible options for including Text now there is only one.

@thisislawatts thisislawatts changed the title chore: move text to components lib chore: drop Text component in favour of Text component May 14, 2022
@thisislawatts thisislawatts changed the title chore: drop Text component in favour of Text component feat: drop Text component in favour of Text component May 14, 2022
@thisislawatts thisislawatts self-assigned this May 14, 2022
@thisislawatts
thisislawatts force-pushed the chore/move-Text-to-components-lib branch from f7475bd to 58983c8 Compare May 14, 2022 16:42
@cypress

cypress Bot commented May 14, 2022

Copy link
Copy Markdown



Test summary

49 0 0 0Flakiness 0


Run details

Project onearmy-community-platform
Status Passed
Commit 56c851c
Started May 15, 2022 1:03 PM
Ended May 15, 2022 1:06 PM
Duration 02:40 💡
OS Linux Ubuntu - 20.04
Browser Chrome 101

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@thisislawatts
thisislawatts force-pushed the chore/move-Text-to-components-lib branch 6 times, most recently from 9645aae to 40b642b Compare May 14, 2022 20:47
@thisislawatts
thisislawatts marked this pull request as ready for review May 15, 2022 08:27
@davehakkens davehakkens added the Review allow-preview ✅ Has received manual check for malicious code and can be safely built for preview label May 15, 2022
@github-actions

github-actions Bot commented May 15, 2022

Copy link
Copy Markdown
Contributor

Visit the preview URL for this PR (updated for commit 56c851c):

https://onearmy-next--pr1663-chore-move-text-to-c-tm3cqfnf.web.app

(expires Wed, 07 Sep 2022 21:11:54 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@thisislawatts
thisislawatts force-pushed the chore/move-Text-to-components-lib branch from 40b642b to 56c851c Compare May 15, 2022 12:55

@davehakkens davehakkens 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.

looks good here!

@thisislawatts

Copy link
Copy Markdown
Contributor Author

@davehakkens Okay to approve these changes?

@davehakkens
davehakkens merged commit 7d2a65c into master May 17, 2022
@davehakkens
davehakkens deleted the chore/move-Text-to-components-lib branch May 17, 2022 16:44
@thisislawatts thisislawatts mentioned this pull request May 17, 2022
3 tasks
@chrismclarke

Copy link
Copy Markdown
Member

Thanks for tidying this, I definitely waas not a fan of the layers of minor wrappers (anything we need to configure with text should be themeable anyways, and also was leading to a bunch of knock-ons passing props like center down which react complains about). I can't remember if we fixed the prop-drilling issue or just avoided react upgrade, but in either case this will definitely help

@onearmy-bot

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.19.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Map 🗺 Research 🧪 Review allow-preview ✅ Has received manual check for malicious code and can be safely built for preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants