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

fix(QIcon): Add check for lowercase 'm' for valid start of svg path #11506

Merged
merged 3 commits into from
Dec 6, 2021

Conversation

hawkeye64
Copy link
Member

@hawkeye64 hawkeye64 commented Nov 27, 2021

Some icon sets work perfectly well with lowercase 'm' and when adding M0 0z causes a black dot in upper-left of the icon. In this case, the regex needs to be /^M|m/

Some icon sets work perfectly well with lowercase 'm' and when adding `M0 0z` causes a black dot in upper-left of the icon. In this case, the regex need to be `/^M|m/`
@hawkeye64
Copy link
Member Author

I have tested this with the change in dev and it works perfectly fine. None of the other icon sets were disturbed.

@rstoenescu
Copy link
Member

/^M|m/ fails when the string contains "m" anywhere inside of it.
Correct is /^[Mm]/ --> pushed commit to fix it.

@rstoenescu rstoenescu merged commit 63c51a3 into quasarframework:dev Dec 6, 2021
@rstoenescu
Copy link
Member

Also, this broke all mdi icons...
Fixed to /^[Mm] ?\d/

@hawkeye64
Copy link
Member Author

I forgot, this also needs to be done for v1

@hawkeye64 hawkeye64 deleted the fix/qicon-svg-path branch December 7, 2021 15:30
@hawkeye64
Copy link
Member Author

In v1, this seems to cause an issue for mdi... icons. Working on a fix.

@hawkeye64
Copy link
Member Author

#11583

I am going to re-review this update (into v2) to make sure there is no conflict with mdi

@rstoenescu
Copy link
Member

Fixed it already. And shipped it in latest Quasar

@onurkose
Copy link
Contributor

onurkose commented Dec 7, 2021

Hi, I have problems rendering simAmazon on Quasar v.2.3.4

const mRE = /^[Mm] ?\d/ is not detecting simAmazon's beginning M.045

I believe it should be like this to support dots right after first character: /^[Mm][ ?\d|.]/

@pdanpdan
Copy link
Collaborator

pdanpdan commented Dec 7, 2021

Of course it was a catch somewhere.
I suppose it could also be +- - I just hope e is not

@hawkeye64
Copy link
Member Author

hawkeye64 commented Dec 7, 2021

Yes, this didn't work out the way intended. Before Quasar update:

image

Quasar v2.3.4 update:

image

The icons are being interpreted as material-icons

image

I'll dig into this.

@hawkeye64
Copy link
Member Author

#11584

@hawkeye64
Copy link
Member Author

Fix is also added into v1: #11583

@haasz
Copy link

haasz commented Dec 19, 2021

Hi,
When will there be a new release with the fix, that is, when will version 2.3.5 (or 2.4.0) be released?

@rstoenescu
Copy link
Member

@haasz It's coming this week. Next Quasar version is a big one, which is why it's taking a bit more time to be released.

@haasz
Copy link

haasz commented Dec 20, 2021

@rstoenescu Thanks for the reply.

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

5 participants