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

dynamically update max chars #51

Merged
merged 1 commit into from
Dec 4, 2021
Merged

Conversation

decanTyme
Copy link
Contributor

@decanTyme decanTyme commented Nov 29, 2021

PR includes:

  • Move normalize.css import to scss
  • Move main stylesheet import from js to html

Summary:

Resolves issue #50:

There should be a function to dynamically calculate maxChar based on at least these contexts: font-family, font-size, and screen dimensions/magnification.

2021-11-29 17-10-03_Trim

Moved main.scss import since it's already bundled in a single stylesheet that can be put directly inside the html, and has no actual use in the js files aside for Parcel reference. Moved normalize.css for better file hierarchy.

Marked as draft since I have not actually tested it yet on other physical screens (only used the browser's devtools) and didn't test other fonts families/styles as well, so please feel free to test!

@vercel
Copy link

vercel bot commented Nov 29, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/warengonzaga/css-text-portrait-builder/AS1NRANCMXfsbfsaHX7B6QuX8DXZ
✅ Preview: https://css-text-portrait-builder-git-fork-decantyme-dev-warengonzaga.vercel.app

@warengonzaga warengonzaga self-requested a review November 29, 2021 10:29
@warengonzaga warengonzaga added feature Issue/Pull Request Label for New Features in progress Issue/Pull Request Label for In Progress Status labels Nov 29, 2021
@warengonzaga warengonzaga modified the milestone: v1.3.0 📦 Nov 29, 2021
@warengonzaga warengonzaga changed the title Dynamically update max chars dynamically update max chars Nov 29, 2021
@warengonzaga
Copy link
Owner

Thanks for the PR @decanTyme! I'll review your update and provide feedback.

@warengonzaga warengonzaga linked an issue Dec 1, 2021 that may be closed by this pull request
@warengonzaga warengonzaga added tweak Issue/Pull Request for Tweaks fix Pull Request Label for any Fixes and removed feature Issue/Pull Request Label for New Features labels Dec 1, 2021
Copy link
Owner

@warengonzaga warengonzaga left a comment

Choose a reason for hiding this comment

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

All approved! Good move and nice implementing best practices.
Let's keep the comments in the JS file. It could help other users to understand the builder.
I'll note this also on the documentation. 😁

* Move `normalize.css` import to scss
* Move main stylesheet import from js to html
@warengonzaga
Copy link
Owner

Are you done with this? @decanTyme

@warengonzaga warengonzaga added waiting response Issue/Pull Request Label for Waiting Response and removed in progress Issue/Pull Request Label for In Progress Status labels Dec 4, 2021
@decanTyme
Copy link
Contributor Author

Are you done with this? @decanTyme

Yep. Already marked as ready. @warengonzaga

@warengonzaga
Copy link
Owner

Awesome, merging this now @decanTyme! Thanks for the awesome contributions!

@warengonzaga warengonzaga merged commit 913d514 into warengonzaga:dev Dec 4, 2021
@warengonzaga warengonzaga removed the waiting response Issue/Pull Request Label for Waiting Response label Dec 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull Request Label for any Fixes tweak Issue/Pull Request for Tweaks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dynamically update max chars
2 participants