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

Tomporvaz/add site button #331

Merged
merged 4 commits into from
Aug 10, 2023
Merged

Tomporvaz/add site button #331

merged 4 commits into from
Aug 10, 2023

Conversation

tomporvaz
Copy link
Contributor

Pull Request 323

Change Summary

Changed the Contribute button text to Add Site
Moved textTransform: none up to button sx

Change Reason

Changed to avoid confusion about contributing information not money.

Verification [Optional]

Phlask_2023-07-18 21-10-56

Related Issue: #323

Copy link
Contributor

@RNR1 RNR1 left a comment

Choose a reason for hiding this comment

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

Looking great overall, just a few styling changes suggested here

<Typography style={{ textTransform: 'none' }} fontSize={'small'}>
Resources
</Typography>
<Typography fontSize={'small'}>Resources</Typography>
Copy link
Contributor

Choose a reason for hiding this comment

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

Curly braces are not necessary for the fontSize prop:

Suggested change
<Typography fontSize={'small'}>Resources</Typography>
<Typography fontSize="small">Resources</Typography>

<Typography style={{ textTransform: 'none' }} fontSize={'small'}>
Filter
</Typography>
<Typography fontSize={'small'}>Filter</Typography>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here:

Suggested change
<Typography fontSize={'small'}>Filter</Typography>
<Typography fontSize="small">Filter</Typography>

<Typography style={{ textTransform: 'none' }} fontSize={'small'}>
Search
</Typography>
<Typography fontSize={'small'}>Search</Typography>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Suggested change
<Typography fontSize={'small'}>Search</Typography>
<Typography fontSize="small">Search</Typography>

}}
>
<ContributeIcon />
<Typography style={{ textTransform: 'none' }} fontSize={'small'}>
Contribute
<Typography noWrap fontSize={'small'}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here:

Suggested change
<Typography noWrap fontSize={'small'}>
<Typography noWrap fontSize="small">

@tomporvaz tomporvaz linked an issue Aug 2, 2023 that may be closed by this pull request
@tomporvaz
Copy link
Contributor Author

@RNR1 I have removed the all of the curly brackets requested, and I also found more in the mobile code. Searching for curly brackets, I realized that the toolbar is duplicated for mobile, so I updated the contribute button on mobile too. I think this is complete, but let me know if there is anything else.

Copy link
Contributor

@RNR1 RNR1 left a comment

Choose a reason for hiding this comment

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

Thanks! Looking good

@RNR1 RNR1 merged commit 991f8f0 into develop Aug 10, 2023
4 checks passed
@RNR1 RNR1 deleted the tomporvaz/AddSiteButton branch August 10, 2023 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the wording on the Contribute Button on Desktop
2 participants