-
Notifications
You must be signed in to change notification settings - Fork 265
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
CSS mirroring support PostCSS, postcss-pxtorem #2016
Conversation
@microsoft-github-policy-service agree |
Thanks a lot for the PR. I've added a couple of reviewers on it. |
Should I add something to the page after the PR is merged ? |
Yes that would be great. The GitHub repository where this page is maintained is: https://github.com/MicrosoftDocs/edge-developer. |
Could you please review my pull request? @vidorteg @ElormCoch @captainbrosset |
thank you for the contribution and for your patience. I left some comment on the associated bug. It will help to clarify the issue this PR is addressing. could you please take a look? |
I'm glad to receive your response. I've also provided a detailed answer and steps to reproduce the issue in the corresponding issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for the detailed steps on how to test the feature, I had some time to review the proposed feature and unfortunately I don't think this is something that we would like to include at the moment. Here is some reasoning behind it:
- The CSS Mirroring feature is designed to be a "what-you-see-is-what-you-get" (WYSIWYG), which is what is happening in here. If you edit the
px
value in the CSS file it will correctly display thepx
in the Devtools. - If you only use
rem
units on both places (source file and Devtools) the WYSISWYG consistency remains intact it is only whenpostcss-pxtorem
plugin kicks-in that it gets broken, which hints that the proposed behavior could likely be achieved as a postCSS plugin (although my expertise in postCSS is limited, so I cannot confirm if this approach is feasible). - On the limited testing I did I found several workarounds, e.g in the
postcss.config.js
file I changedreplace: false
which kept both values (px and rem) in the file and in the Devtools (which is consistent with WYSISWYG).
Thank you for using the Edge Devtools extension.
@vidorteg Thank you very much for your thoughtful review and feedback on my PR. I greatly appreciate the time you've invested in evaluating the proposed CSS Mirroring feature and understand the concerns you've raised regarding its integration and the potential impact on the WYSIWYG principle, especially in relation to the postcss-pxtorem plugin. I would like to clarify an important aspect of the implementation that might not have been fully apparent: the feature, including the vscode-edge-devtools.postCSSRootValue configuration option, had already been implemented prior to your comment. This configuration ensures that the feature is activated only when users explicitly set it, thereby maintaining the integrity of the Edge Devtools extension's default behavior and offering flexibility for developers to opt-in based on their project requirements. This approach was designed with the intention of preserving the WYSIWYG consistency for users who do not utilize the postcss-pxtorem plugin, while also providing a valuable tool for those who do, particularly in the context of mobile web development. By making the feature's activation a deliberate choice, we aim to cater to the diverse needs of our user base without disrupting the core user experience of the Devtools. The necessity of this feature arises from the specific challenges faced by mobile web developers, who require precise control over responsive design elements across various devices and screen resolutions. The ability to convert px to rem units seamlessly is crucial for developing accessible, responsive web applications. Feedback from the mobile web development community indicates a strong demand for such functionality, underscoring its potential value to our toolset. I hope this clarification helps address some of the concerns you've raised. I am fully committed to ensuring that this feature aligns with the project's goals and standards and am open to further discussions, testing, or revisions as needed. Your feedback is invaluable to refining and improving our contributions to the vscode-edge-devtools project. Thank you once again for your consideration and for the opportunity to contribute. I look forward to any additional thoughts you may have and am eager to work together towards enhancing the utility and versatility of the Edge Devtools extension. |
Indeed, setting replace: false in the postcss.config.js file, while keeping both px and rem values in the file and in Devtools, does not actually solve the issue, as it results in both px and rem values being written simultaneously, which is not the intended solution. |
Certainly, it's possible to achieve the desired outcome through alternative methods. If deemed unnecessary, feel free to close this PR. |
#2010