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

Feature / Font size selector on PDF export #1612

Conversation

chelsouse
Copy link

PR for issue #1575

Copy link
Collaborator

@RodriSanchez1 RodriSanchez1 left a comment

Choose a reason for hiding this comment

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

@chelsouse Nice work!! Take a look at the comments that i made! Let me now if you have some doubts!

src/components/Settings/Export/Export.component.js Outdated Show resolved Hide resolved
src/components/Settings/Export/Export.component.js Outdated Show resolved Hide resolved
src/components/Settings/Export/Export.component.js Outdated Show resolved Hide resolved
src/components/Settings/Export/Export.helpers.js Outdated Show resolved Hide resolved
src/components/Settings/Export/Export.helpers.js Outdated Show resolved Hide resolved
@chelsouse
Copy link
Author

Hello @RodriSanchez1

I've implemented the requested changes. Please let me know, if it needs any futher changes.

Copy link
Collaborator

@RodriSanchez1 RodriSanchez1 left a comment

Choose a reason for hiding this comment

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

@chelsouse Nice workk! I made a few changes, check it out!

I want to give you a little advice, in your future commits try to implement atomic commits. This way, your work will be more detailed. An also always add a PR description!

See:
What's an atomic git commit?
Working with atomic git commits means your commits are of the smallest possible size. Each commit does one, and only one simple thing, that can be summed up in a simple sentence.
The amount of code change doesn't matter. It can be a letter or it can be a hundred thousand lines, but you should be able to describe the change with one simple short sentence.

Hope you want continue contributing on Cboard!
Regards, Rodri!

@RodriSanchez1 RodriSanchez1 merged commit 83406db into cboard-org:master Nov 16, 2023
5 checks passed
@chelsouse
Copy link
Author

Hello @RodriSanchez1

Thanks for your help and advice on atomic commits! It's my first PR, and I'll definitely keep your suggestions in mind. Appreciate the guidance!

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.

None yet

2 participants