-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-10730] Override FireFox extension styles for text alignment #10528
Conversation
@shane-melton @Jingo88 There doesn't appear to be code owners, feel free to add who you see fit! |
New Issues
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #10528 +/- ##
=======================================
Coverage 32.78% 32.78%
=======================================
Files 2669 2669
Lines 81514 81514
Branches 15355 15355
=======================================
Hits 26727 26727
Misses 52743 52743
Partials 2044 2044 ☔ View full report in Codecov by Sentry. |
Nice find on this! After looking into some more myself, I think we be able to just set It already defaults to I think this has the benefit of avoiding manually overriding the styles and having to track why its there (granted, your comment is very clear). It also moves us one small step closer to future MV3 defaults. |
- which solves the text-alignment issue in FF
…t/pm-10730/firefox-text-alignment
@shane-melton Nice find! I remember reading through that but I have no idea why I ruled it out in my head. Either way, tested and functions as we'd expect! 👏 |
QA Approval given on ticket 🥳 |
🎟️ Tracking
PM-10730
📔 Objective
This one is probably worth discussing as it is a global change.
Issue
FireFox applies default styles to extensions, one of which is
text-align: start
. This causes some elements to not inherit thetext-align
property from their parent, as they would in other browser implementations.You can see the conflicting styles here and an explanation of the entire stylesheet here.
Fix
Rather than playing wack-a-mole with each implementation in the bug ticket, and any other future bug tickets for the same (like PM-8626). I applied a global style for the extension that will override the FF styles and force inheritance as the default behavior for thetext-align
property.Edit: See Shane's comment below: #10528 (comment)
The comment within the file also states that these styles will be going way for MV3. PM-8317 is out there to do this work in the future.
Of course an option is to find each conflicting implementation and add
tw-text-center
but that isn't as future proof. I'm happy to do that, but wanted to start here first!📸 Screenshots
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes