This CL migrates the CSSOverviewCompletedView.css file to remove any
colour patching.
I've left some colours alone:
1. The icon colours (red/green) because these work fine across both
light and dark mode.
2. The slight box shadow on the bars because again these work fine. I
suspect longer term we might want to define some default shadows for
each theme.
Bug: 1152736
Change-Id: I3959e5e01007541da91b7f91906d74df4008d972
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2560251
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
This CL fixes the issues outlined in the ticket:
- Resets the state when the CSS overview is closed to
prevent the elements from going missing and to synchronise the
scroll state with the selected tab.
- Sets the min-width on the contrast column elements to prevent cramping.
Fixed: 1152692
Change-Id: I66f4ad1b04bb6c64251a8c73ea645fbc125446f0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2560235
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
ui/Tooltip.js overrides the HTMLElement prototype to override the
title property. Rather than using this property, we shoul be
calling Tooltip.install directly. This makes sure that new
components are not relying on the behavior of the legacy
prototype patching.
These usages have been manually audited using the following regexes:
Search: ([\S]+)\.title = ([^;]+);
Replace: UI.Tooltip.Tooltip.install($1, $2);
Note that there are classes in DevTools that also have a title
property. Most notably `TreeElement`. We should not be replacing
these, as they do not inherit from HTMLElement. Luckily, we are
running TypeScript to make sure we don't call `Tooltip.install`
with a non-HTMLElement.
A follow-up CL will clean up the getters.
R=jacktfranklin@chromium.org
Bug: 1150762
No-presubmit: True
Change-Id: I5928e75c70293531849e0576f4fb2a2a8b3e02d2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2555060
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
As part of the dark mode migration we are adding
`enableLegacyPatching:true` to all call sites of `appendStyle` (and
related methods). If a user passes `false` for that flag, no CSS colour
patching will occur.
This CL is therefore a no-op from a user's perspective as we pass `true`
on every call to maintain existing behaviour, but once we start
migrating we will turn the option to `false`. Long term, once all code
is migrated and the old patching is removed, we will remove this flag
entirely once again.
Bug: 1122511
Change-Id: I76b818e83b7d5ee0175e1548759373f53481e0ab
No-Presubmit: True
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2516345
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
The Target.model method's type can be improved to convey the type of
the result (based on the constructor argument). This CL does just that.
This unlocks more checking down the line. The CL also alters the types
of the observeModel(s) methods, but in a weaker way, as closure is
apparently not able to instantiate the template correctly for method
invocations.
Bug: chromium:1011811
Change-Id: Ic466cfd946c30f3fcfb4ce24a8a0e1bc542e7d3f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2445776
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Lexical declarations in `case` and `default` clauses are a footgun,
since they are visible in the entire switch block, but they only
get initialized upon assignment, which only happens if the relevant
`case` is actually reached.
To ensure that such lexical declarations only apply to the current
`case` (which is usually the intention), `case` clauses containing
them should be wrapped in curly braces to create an explicit block.
More information:
https://eslint.org/docs/rules/no-case-declarations
Change-Id: I63d9341fcd76d4b9ce8281bd0e6573b886577f08
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2119685
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
We prefer single quotes by default, but still want to allow double
quotes in cases where that helps avoid quote-escaping, and also want
to allow template literals in cases where string interpolation is
used. For example:
const a = 'xxx'; // ok
const b = "xxx"; // not ok, should use single quotes
const c = "xxx'xxx"; // ok, double quotes avoid an escape sequence
const d = `xxx`; // not ok, frivolous use of template literal
const e = `xxx ${42}`; // ok
Per https://github.com/eslint/eslint/issues/12976, setting
the `allowTemplateLiterals` option for the `quotes` lint rule to
`false` gives us the desired behavior.
Note that the above lint rules are auto-fixed when running the linter;
there should be no need to manually make any changes to appease the
linter.
Per review feedback, this patch also removes the following escape
sequences for printable non-ASCII symbols:
- U+00D7 → ×
- U+2026 → …
- U+2019 → ’
Chromium CL temporarily updating test expectations:
https://chromium-review.googlesource.com/c/chromium/src/+/2083147
Cq-Depend: chromium:2083147
Bug: chromium:1057042
Change-Id: Id6bec3f96ca694d2fbc07dd8629fce305a58df8a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2082372
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Mathias Bynens <mathias@chromium.org>
Issue:
- Data grids in DevTools do not have an aria-label
- There are a ton of DataGrid parameters and adding one is messy
Changes:
- Naming all data grids
- Created DataGrid.Datagrid.Parameters type to hold data grid parameters (including a required gridName field)
Bug: 963183
Change-Id: I83b130d468fb80034be264b45b86b799f650e978
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1891652
Commit-Queue: Michael Liao <michael.liao@microsoft.com>
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
This is the first pass at adding unused rules to the CSS Overview.
The CL adds support for checking width & height to inline elements
as well as position values to static elements. Further CLs will
add additional checks for other cases, e.g. the use of flex or grid
values against non-flex and non-grid elements respectively.
Change-Id: I2fb4fd557d67065a5753587624195920c22b3edc
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1886818
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
The Chromium/Google style guides does not enforce curly braces for
single-line if-statements, but does strongly recommend doing so. Adding
braces will improve code readability, by visually separating code
blocks. This will also prevent issues where accidental additions are
pushed to the "else"-clause instead of in the if-block.
This CL also updates the presubmit `eslint` to run the fix with the
correct configuration. It will now fix all issues it can fix.
Change-Id: I4b616f21a99393f168dec743c0bcbdc7f5db04a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821526
Commit-Queue: Tim Van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jeff Fisher <jeffish@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#701070}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7e0bdbe2d7f9fc2386bfaefda3cc29c66ccc18f9
Tim van der Lippe
