Skip to content

refactor(styles): Migrate LabelButton#613

Closed
josenaranjo wants to merge 1 commit intogitpoint:masterfrom
josenaranjo:refactor-label-button
Closed

refactor(styles): Migrate LabelButton#613
josenaranjo wants to merge 1 commit intogitpoint:masterfrom
josenaranjo:refactor-label-button

Conversation

@josenaranjo
Copy link
Copy Markdown
Contributor

Question Response
Version? v1.4.0
Devices tested? iPhone 7
Bug fix? no
New feature? no
Includes tests? no
All Tests pass? no
Related ticket? #532

LabelButton's tests are not passing, I already tried to fix it, but didn't find the reason why it is failing. Please let me know if you have any hint.


Screenshots

Before After
![before](image  ![after](image
)

Description

Refactored LabelButton to render a LabelButton large or small depending on largeWithTag prop

Refactored LabelButton to render a LabelButton large or small depending on largeWithTag prop

gitpoint#532
@chinesedfan
Copy link
Copy Markdown
Member

@josenaranjo I am afraid that it is better to keep as one component and control properties by props. For example,

const LabelButton = styled.Button`
    font-size: ${props => props.largeWithTag ? 13 : 12};
`;

Copy link
Copy Markdown
Member

@machour machour left a comment

Choose a reason for hiding this comment

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

@josenaranjo The provided screenshot doesn't showcase the LabelButton component at all.

LabelButton can be seen in a existing issue or pull request setting screen (use the cog icon to access it).

Additionally, you are dropping the off react-native-elements and using the Button from react-native, which is probably not what you want to do.

Don't hesitate if you need any help buddy :)

@machour
Copy link
Copy Markdown
Member

machour commented Jan 15, 2018

Hi @josenaranjo, friendly ping, are you still up for this one? Let us know 🙏

@chinesedfan
Copy link
Copy Markdown
Member

@josenaranjo Sorry, #727 will take over your work. But please feel free to help our project at any time in the future!

@machour
Copy link
Copy Markdown
Member

machour commented Feb 23, 2018

#727 was merged in, closing this stale PR

@machour machour closed this Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants