Skip to content

ui: CSS modules for PlanView component#47513

Merged
craig[bot] merged 3 commits intocockroachdb:masterfrom
koorosh:koorosh/ui-statements-extraction--plan-view
Jun 4, 2020
Merged

ui: CSS modules for PlanView component#47513
craig[bot] merged 3 commits intocockroachdb:masterfrom
koorosh:koorosh/ui-statements-extraction--plan-view

Conversation

@koorosh
Copy link
Copy Markdown
Contributor

@koorosh koorosh commented Apr 15, 2020

Depends on: #47484
Related to: #47527

  • Refactored font imports to correctly resolve paths when module is required from different locations;
    Fonts are imported directly from app.styl file which allows import typography.styl without dependencies. This change was required because importing typography.styl file from CSS modules
    failed with unresolved paths inside of fonts.styl file (which was required in typography.styl file).
    Before,
app.styl
|-- typography.styl
     |-- fonts.styl

Now:

app.styl
|-- typography.styl
|-- fonts.styl
  • Move all files related to PlanView component under planView directory
  • Added storybook for PlanView component
  • planView.module.styl file contains copy of styles (from statements.styl) which is used by component only.

Release note: None

@koorosh koorosh added the do-not-merge bors won't merge a PR with this label. label Apr 15, 2020
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Apr 15, 2020

❌ The GitHub CI (Cockroach) build has failed on f5c48070.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch from f5c4807 to e57229c Compare April 15, 2020 14:36
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Apr 15, 2020

❌ The GitHub CI (Cockroach) build has failed on e57229c4.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch from e57229c to 34de871 Compare April 23, 2020 14:10
@koorosh koorosh changed the title ui: WIP. Statements extraction for PlanView component ui: Statements extraction for PlanView component Apr 23, 2020
@koorosh koorosh marked this pull request as ready for review April 23, 2020 14:25
@koorosh koorosh requested a review from a team April 23, 2020 14:25
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Apr 23, 2020

❌ The GitHub CI (Cockroach) build has failed on 34de8711.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch from 34de871 to ac7e6c5 Compare April 24, 2020 08:40
@koorosh koorosh changed the title ui: Statements extraction for PlanView component ui: CSS modules for PlanView component Apr 24, 2020
@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch 2 times, most recently from a70787d to 2007146 Compare April 28, 2020 07:01
@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch 2 times, most recently from af4d5a8 to cccd35c Compare May 19, 2020 13:20
@koorosh koorosh removed the do-not-merge bors won't merge a PR with this label. label May 19, 2020
@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch 2 times, most recently from 9c7afcf to 3ab52bb Compare May 22, 2020 08:19
@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch 2 times, most recently from a312e4b to aae59a3 Compare June 2, 2020 09:18
koorosh added 3 commits June 4, 2020 11:24
To test styles isolation for statements page
we need storybook which displays entire
Statements screen only.

To make it work, RouterProvider decorator
is added which connects router to dummy (empty)
store.

`statementsPage.fixture.ts` file contains snapshot
of required props for StatementsPage component

Release note: None
- refactor fonts imports to correctly resolve paths
when module is required from different locations;
- move all files related to PlanView component under
`planView` directory
- Added story for PlanView component

Release note: None
Previously, class names were constructed by simply accessing
style modules class names and assigning it to classes. It was
cumbersome and not readable at all.

To enhance this, `classnames/bind` alternate is used, which
allows simply put class names.

Release note: None
@koorosh koorosh force-pushed the koorosh/ui-statements-extraction--plan-view branch from aae59a3 to 5fff70b Compare June 4, 2020 08:25
@koorosh
Copy link
Copy Markdown
Contributor Author

koorosh commented Jun 4, 2020

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jun 4, 2020

Merge conflict (retrying...)

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jun 4, 2020

Build succeeded

@craig craig bot merged commit 82f172a into cockroachdb:master Jun 4, 2020
craig bot pushed a commit that referenced this pull request Jun 4, 2020
47606: ui: CSS modules for Table components r=koorosh a=koorosh

Depends on #47513
Related to #47527

This change refactors components to use CSS modules and
incorporate all required styles without any external
dependencies and prevent styles altering from outside.
It affects several components which tightly
coupled with StatementsTable and couldn't be changed
separately.

Following component are changed:
- HighlightedText
- Drawer
- StatementsTable
- SortableTable

Note, that `StatementsTable#makeCommonColumns` function
is refactored to provide custom styles from parent to
child components via props instead of overriding styles.

Storybook is extended to show some components as independent
units or in context of `StatementTable` component (if it is
only the way components work).

Release note: None

49770: changefeedccl: change default flush interval to 5s r=dt a=dt

We observed a customer cluster's changefeeds to cloud storage 'getting stuck'
which on further investigation was determined to be happening because they
were spending too much time in flushing. This was because they were writing to
a cloud sink and the default flush interval of 200ms (poller interval of 1s / 5)
meant it spent all of its time flushing. This default was picked testing with
lower-latency sinks and was noted in a comment as somewhat arbitrary.

This change does two things: it increases the default to the poller interval
if unspecified, instead of poller interval / 5, meaning 1s instead of 200ms
at the default setting, and if the sink being used is cloud storage, it
changes it to the greater of that or 5s. Users who truely desire lower latency
can of course specify their own 'resolved' interval, so this change in the
default is for those that are indifferent, and increasing the latency to 1s or 5s
reduces the chance of hiitting this unfortunate edge case when the sink is too slow.

Release note (enterprise change): The default flush interval for changefeeds that do not specify a 'resolved' option is now 1s instead of 200ms, or 5s if the changefeed sink is cloud-storage.

49819: Use faster set for column IDs in schemaexpr r=mgartner a=mgartner

#### sqlbase: add ColSet wrapper for util.FastIntSet of ColumnID

There are numerous places where a `map[sqlbase.ColumnID]struct{}` or a
`util.FastIntSet` is used to represent a set of `sqlbase.ColumnID`. This
commit adds a typed wrapper around `util.FastIntSet` which is an
efficient and ergonimic replacement for maps and `util.FastIntSet`.

Release note: None

#### schemaexpr: use sqlbase.ColSet instead of maps

This commit replaces maps used as sets of integers with sqlbase.ColSet
because it is a more efficient set implementation.

Release note: None


Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>
Co-authored-by: David Taylor <tinystatemachine@gmail.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants