fix: Update the box component to Warp using component-classes #14
Conversation
…ater and restructured the document
…iants and options
There was a problem hiding this comment.
Nice stuff! Had two small comments but other than that it looks alright!
It would be really helpful if a PR had just the changes that are relevant based on the title/description. Refactoring or changes in other parts of the code base increase "noise" and when it's a new syntax for one's eyes (like vue's is for mine), it extends the review time a bit 😊
| <template> | ||
| <div class="f-expandable"> | ||
| <div class="will-change-height"> |
There was a problem hiding this comment.
I see that "will-change-height" classname is assigned to component-classes' expandable object. Not sure it makes sense to apply it here, too, but thought I'd mention it.
There was a problem hiding this comment.
Not sure if we really want to use it at all after having read this (and other things) in MDN:
Don't apply will-change to elements to perform premature optimization. If your page is performing well, don't add the will-change property to elements just to wring out a little more speed. will-change is intended to be used as something of a last resort, in order to try to deal with existing performance problems.
😄
But I think we will just keep it for now?
There was a problem hiding this comment.
Sounds like something we could add to the "will improve later" task pile 😄 I don't think we have a pile like that ready in Jira, so maybe add a TODO comment for now? We can eventually scan our repos to find all TODOs and then create tasks based on those.
Still, the question remains: what about using ccExpandable.expandable instead of a hard-coded utility class? What do you think about that?
There was a problem hiding this comment.
Ah ok so NOW I understand the question. 😆 😅
For some reason I think this only exists in Vue... So maybe it would be ok just to use the class from ccExpandable here. 👍
There was a problem hiding this comment.
Ah no... at second thought, that wouldn't be right. 😄
If you decide to add more classes to or remove will-change-height from ccExpandable that too would affect this component and that might not be the desired result.
As this is only in Vue I think it would be better just to leave it as it is, with a todo comment, and not pollute the component-classes with this by adding it just because of the use here in Vue, that might not be correct? 😄 Agree? 🙂
| <template> | ||
| <div class="f-expandable"> |
There was a problem hiding this comment.
I found that f-expandable class sets -webkit-tap-highlight-color: transparent; on the element. Not sure it's still needed, as it's a non-standard CSS property, but should we maybe check if it's safe to remove it?
There was a problem hiding this comment.
It would be really helpful if a PR had just the changes that are relevant based on the title/description. Refactoring or changes in other parts of the code base increase "noise" and when it's a new syntax for one's eyes (like vue's is for mine), it extends the review time a bit 😊
I think it's ok at this point to fix small things when we see them, just to progress faster...? But I agree with you that's better... at least when we are at the point of a more "stable" product. 😄
Anyways I should have added a description so that it was clearer. 👍🙂
(I am used to getting all the commit messages automatically in the description as default... that would have been handy here. 😄)
…s import with the project naming pattern
# [1.0.0-alpha.7](v1.0.0-alpha.6...v1.0.0-alpha.7) (2023-04-18) ### Bug Fixes * Update the box component to Warp using component-classes ([#14](#14)) ([eab3eb1](eab3eb1))
# 1.0.0 (2023-08-31) ### Bug Fixes * add button classes ([#7](#7)) ([192f983](192f983)) * add classes to card component ([#31](#31)) ([acebdc3](acebdc3)) * add classes to clickable ([#29](#29)) ([c220593](c220593)) * add classes to radiobuttons component ([#27](#27)) ([1520b7a](1520b7a)) * add classes to textfield and textarea ([#13](#13)) ([d07be64](d07be64)) * add component classes to breadcrumbs ([#19](#19)) ([0a44566](0a44566)) * add component classes to modal ([#33](#33)) ([a2dfc25](a2dfc25)) * add component classes to select ([#18](#18)) ([99f5691](99f5691)) * add component classes to the toggle component ([#24](#24)) ([4526148](4526148)) * add default extractor (handles .js files) + add translation for validation ([4b32bbb](4b32bbb)) * add Finnish language ([431c62b](431c62b)) * add more translations ([2f06c7e](2f06c7e)) * Add uno styling for the alert component ([#6](#6)) ([50c21f6](50c21f6)) * **alert:** use imported instead of inline classes ([#39](#39)) ([832521b](832521b)) * bump @warp-ds/css package to get fix for select chevron ([675f07c](675f07c)) * bump component-classes ([60940a4](60940a4)) * bump fversions ([#45](#45)) ([18941af](18941af)) * bump packages ([#40](#40)) ([cd4b4ae](cd4b4ae)) * bump warp packages ([#47](#47)) ([9ff62db](9ff62db)) * **buttons:** bump component-classes to 1.0.0-alpha.116 ([#51](#51)) ([6f077c4](6f077c4)) * detect locale on server ([68ceedc](68ceedc)) * do not override props and attrs in input and textarea ([#17](#17)) ([e549203](e549203)) * **expandable:** add the same settings for info as for box prop + rename import of component classes ([1c85e36](1c85e36)) * Fixed bug with Switch always showing selected state ([#37](#37)) ([0fab7b3](0fab7b3)) * focusable is added to the component classes ([#38](#38)) ([02e5df2](02e5df2)) * import sr-only class from component-classes to Button and Breadcrumbs ([#20](#20)) ([45e67a6](45e67a6)) * **inline classes:** update inline classes for tag, tabs, steps, expandable, attention ([#46](#46)) ([71feb82](71feb82)) * load messages correctly ([b0cbf19](b0cbf19)) * modify test because of changed default aria label ([ec7579f](ec7579f)) * **package.json:** update package versions ([#48](#48)) ([d5ae45c](d5ae45c)) * **package.json:** update version for component-classes in package.json ([658c232](658c232)) * provide props to textarea ([#41](#41)) ([f5a37f1](f5a37f1)) * remove inline class from button link ([#43](#43)) ([008b445](008b445)) * Rename wInput to wTextfield and add prop validator for input types ([#58](#58)) ([9d667e2](9d667e2)) * revert "chore(release): 1.0.0-alpha.44 [skip ci]" ([d03b908](d03b908)) * stable FINN prod alpha-release ([#50](#50)) ([73b9d6d](73b9d6d)) * **stepindicator:** update version of @warp-ds/component-classes ([7914c87](7914c87)) * **steps:** fix background color issue for active step ([#42](#42)) ([1e6c7b2](1e6c7b2)) * **steps:** small fix ([836eef8](836eef8)) * **steps:** update to computed classes ([97f3b4d](97f3b4d)) * textarea default class and refactor ([#15](#15)) ([e64455d](e64455d)) * Update the box component to Warp using component-classes ([#14](#14)) ([eab3eb1](eab3eb1)) * Update to use first major versions of the css and uno warp packages ([#59](#59)) ([69bd395](69bd395)) * use css package instead of component-classes ([#52](#52)) ([625d061](625d061)) * use props for inputs and not attrs ([#26](#26)) ([ec08a63](ec08a63)) * use radioChecked class ([#28](#28)) ([e0e7200](e0e7200)) * use updated component-classes in alert ([#8](#8)) ([59227cf](59227cf)) * **w-pill:** minor changes to clean it up ([2181a74](2181a74)) * Warpified and refactored the Pill component, adding component-classes ([#22](#22)) ([3e888ae](3e888ae)) * **workflows:** set node v to lts/* and pnpm to 8 ([#44](#44)) ([c4ea6ba](c4ea6ba)) ### Features * add internationalization (i18n) ([5cb0387](5cb0387)) * **attention:** warpify attention ([#34](#34)) ([6aedb69](6aedb69)) * **button-group:** add component classes to ButtonGroup ([#23](#23)) ([c9e29a9](c9e29a9)) * **Form:** warpify forms ([#36](#36)) ([f712528](f712528)) * **slider:** warpify slider component ([#30](#30)) ([67efc75](67efc75)) * **stepindicator:** add component classes to step indicator ([de3474d](de3474d)) * **tabs:** Warpify tabs ([#32](#32)) ([2e996aa](2e996aa)) * Warpify Switch component ([#35](#35)) ([26c03fa](26c03fa)) * **workflows:** add eik aliasing script to release.yml ([#10](#10)) ([a0de9d6](a0de9d6)) ### BREAKING CHANGES * wInput has been renamed to wTextfield. This change is made to align with the other frameworks in Warp and to clarify what type of inputs the component produces.
No description provided.