Closed Chicken closed 2 months ago
Is there some reason to not support an array of string matches? I feel like it allows for easy matching when a simple string match is hard to figure out and probably performs better compared to a RegExp. It also doesn't take much code to support.
Keeping the old patches is quite brilliant, no idea why I didn't think of that. Though, would probably need an almost duplicate component as the new layout required quite different styles and writing that as conditionals would be way too messy to read.
Though, would probably need an almost duplicate component as the new layout required quite different styles and writing that as conditionals would be way too messy to read.
if its just a different class, you can use our classes utility:
className={classes({
[styles.old]: !isNewProfile,
[styles.new]: isProfile
})}
as for performance of array vs regex, as long as you keep the regex simple, performance won't be a big deal. if anything, using an array find might be slower because it has to walk the same string multiple times
it's best to keep the find as simple as possible and regex should cover everything you need
if its just a different class
Well it did change from a div to a section, different class name, different heading variant and also a different amount of wrapper divs.
For the RegExp I would just use something like /onOpenProfile.+subsection.+USER_PROFILE_MEMBER_SINCE/
(Basicly just converting the array into a RegExp because I have no idea how to figure out nice matchers)
Hei Antti!
You can do either bio)!==""&&
or .PANEL}),nicknameIcons
as a find :)
Removed all the array stuff, used suggested find above, used the more performant find function, kept old patches, put the sidebar stuff in a boolean prop but kept new and old layouts as separate components. Removed the old layout css file and classes though as the css file was never imported and used (?)
maybe it would look better in one line? wastes a lot of space being separate
or even like this
Well, I agree. But the point of this PR is to just make it work like it used to. That can be looked into later.
thanks for the fix!
I'm actually not sure if this should be merged yet, I don't think the new user popups and profiles are pushed out for everyone yet (?) Added matching modules with a string array because I couldn't figure out a good single string match and using RegExp felt too overkill. Added myself to the authors because felt this was a big enough contribution.