Sitelet https://web.archive.org/web/20211221151714/https://github.com/NativeScript/NativeScript/pull/9110
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

Jump to bottom

feat(color): added utilities and improved color parsing performance #9110

Merged
merged 24 commits into from Aug 11, 2021
Merged

feat(color): added utilities and improved color parsing performance #9110

merged 24 commits into from Aug 11, 2021

Conversation

@farfromrefug
Copy link
Collaborator

@farfromrefug farfromrefug commented Dec 24, 2020

This is second part of my last PR about adding features to Color. It actually can replace the last PR. And it does more:

  • add new utilities function taken from tinycolor
  • make Color much faster. Running profiling in my apps i realized Color was pretty slow.
    • hex method was dead slow. _argb is now used to create the hex string.
    • all the regexp tests were actually unnecessary as it would throw an error anyway if the color was not formatted correctly.
@farfromrefug
feat: color methods
44ee97b 
Should fill most needs and remove the need for external libs like tinycolor
@farfromrefug
typo fix
aa9c5d1
@farfromrefug
fix: improve Color performances.
4ef99ab 
* no need to constantly test for SHARP
* drop the regexp test. Testing for first char SHARP is enough as it will still throw an error if the color is not valid
* faster hex method thus also toString method
@farfromrefug
fix: fix regression after colors simplifications
5a61587
@farfromrefug
Merge branch 'color_update' of github.com:Akylas/NativeScript into co…
dc740e3 
…lor_changes_2
@farfromrefug
fix: added Color.mix
619a3f0 
Fixed hsl and hsv methods
@farfromrefug
chore: mix fonction fix
3728e68
@cla-bot cla-bot bot added the cla: yes label Dec 24, 2020
@NathanWalker NathanWalker changed the base branch from master to release/8.0.0 Jan 29, 2021
@NathanWalker NathanWalker changed the base branch from release/8.0.0 to master Apr 18, 2021
@NathanWalker
Copy link
Contributor

@NathanWalker NathanWalker commented Apr 18, 2021

@farfromrefug when get a chance possible to resolve the conflicts and we could get this into 8.1?

@farfromrefug
Copy link
Collaborator Author

@farfromrefug farfromrefug commented Apr 20, 2021

@NathanWalker i want to but i don't understand some changes you made when you merged the first one.
like for example your brought back _normalizeHex when it is not necessary and takes time.

const hex = this._normalizeHex(arg);
this._argb = this._argbFromString(hex)

Same thing with hex getter. You splitted the components instead of simply using toString(16). Yes we could end up with #fff but do we really care? It is faster.

I ll fix the rest when we ve decided on this.

@farfromrefug
Copy link
Collaborator Author

@farfromrefug farfromrefug commented May 22, 2021

@NathanWalker i just realized that it was a bit worse that i thought. The changes you made when you rewrote my original PR actually broke most of the colors functions (released in 8.x).
That PR (once i force the resolves) fixes them back. I will check the linting but please accept it as it is or test it if you rewrite it.

farfromrefug added 2 commits Jun 9, 2021
@farfromrefug
fix: color hex getter fix
aa0f334 
# Conflicts:
#	packages/core/color/color-common.ts
@farfromrefug
Merge branch 'master' of github.com:NativeScript/NativeScript into co…
cb2326d 
…lor_changes_2
@farfromrefug
Copy link
Collaborator Author

@farfromrefug farfromrefug commented Jun 9, 2021

@NathanWalker i updated the PR and fixed everything. AS a note currently in N all color functions based on hsl/hsv are broken

@NathanWalker NathanWalker added this to the 8.1 milestone Jul 12, 2021
@farfromrefug
Copy link
Collaborator Author

@farfromrefug farfromrefug commented Jul 12, 2021

@NathanWalker I still need to push one last fix on the hex getter

@farfromrefug
Copy link
Collaborator Author

@farfromrefug farfromrefug commented Aug 7, 2021

@NathanWalker this PR really fixes a lot. I just added a fix for linear-gradient parsing of hex colors with the new #rrggbbaa format

@farfromrefug
Copy link
Collaborator Author

@farfromrefug farfromrefug commented Aug 7, 2021

@NathanWalker do we have tests for linear-gradient parsing ? (Not in front of my computer). I changed lot to refactor and handle all colors parsing in Color class. Would be good to ensure all that is good

@NathanWalker NathanWalker changed the base branch from master to release/8.1.0 Aug 11, 2021
@NathanWalker NathanWalker changed the title Color changes 2 feat(color): added utilities and improved color parsing performance Aug 11, 2021
@NathanWalker NathanWalker merged commit 0ff2221 into NativeScript:release/8.1.0 Aug 11, 2021
2 of 3 checks passed
NathanWalker added a commit that referenced this issue Sep 8, 2021
@farfromrefug @NathanWalker
feat(color): added utilities and improved color parsing performance (#…
c4db847 
…9110)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NathanWalker NathanWalker

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
8.1
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@farfromrefug @NathanWalker