Skip to content
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

Update dataviews package version to latest #93503

Merged
merged 102 commits into from
Sep 4, 2024

Conversation

allilevine
Copy link
Member

@allilevine allilevine commented Aug 13, 2024

Fixes https://github.com/Automattic/jetpack-manage/issues/337

Proposed Changes

  • Update dataviews package version.
  • Remove locks on the version changes.
  • Downstream changes to use the new package version on the Hosting Dashboard (/sites), the A4A Dashboard, and the Add Site > Via WordPress.com modal.
Before After
Screen Shot 2024-09-03 at 11 32 16 AM Screen Shot 2024-09-03 at 11 31 39 AM

Why are these changes being made?

  • To update to the latest version of DataViews in Core. See pe7F0s-21J-p2

Status

Status DataViews instance URL
🟢 Dotcom Sites <calypso>/sites
🟢 A4A Sites <agencies>/sites
🟢 A4A Sites Modal <agencies>/sites ("add new site" modal)
🟢 A4A Referrals Overview <agencies>/referrals
🟢 A4A Referrals Details <agencies>/referrals (with an item selected)
🟢 A4A Referrals for clients <agencies>/clients/subscriptions
🟢 A4A Team (in development, not in production) <agencies>/team
🟢 Jetpack Cloud Sites (legacy, not in production): one, two. <jpcloud>/sites

Dotcom Sites

  • Nothing to fix in this PR.

A4A Sites

  • Row notice for sites whose status is "Backup failed".

A4A Sites Modal

  • Nothing to fix in this PR.

A4A Referrals Overview

  • Nothing to fix in this PR.

A4A Referrals Details

  • Nothing to fix in this PR.

A4A Referrals for clients

  • Infinite data loop. There's a fix but not sure if it's the right one (conversation).
  • Actions column name is not aligned to the right as it was before. This is because existing CSS targets data-field-id="actions" that no longer exists in DataViews.

A4A Team

  • Nothing to fix in this PR.

Jetpack Cloud

  • These are not in use, they should be removed from the codebase.

Testing Instructions

  • Check out this branch locally, or use the Calypso live links below.
  • Open /sites. Verify that:
    • the list of sites loads and has pagination.
    • you can click on a site and open the fly out panel, and the table collapses into a list. The sidebar collapses into a rail.
    • you can search and filter the sites.
    • the table loads only Site and Actions on mobile.
  • Open the Agencies dashboard. Verify that:
    • the list of sites loads and has pagination.
    • you can click on a site and open the fly out panel, and the table collapses into a list.
    • you can search and filter the sites.
    • you can favorite sites.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@allilevine allilevine changed the title Update dataviews package version. Update dataviews package version to latest Aug 13, 2024
@matticbot
Copy link
Contributor

matticbot commented Aug 13, 2024

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~2217 bytes removed 📉 [gzipped])

name                   parsed_size           gzip_size
entry-main                 +3814 B  (+0.2%)     -358 B  (-0.1%)
entry-stepper              +2684 B  (+0.1%)     -486 B  (-0.1%)
entry-subscriptions         -755 B  (-0.0%)    -1819 B  (-0.4%)
entry-login                 -308 B  (-0.0%)     -223 B  (-0.1%)
entry-domains-landing       -308 B  (-0.0%)     -223 B  (-0.1%)
entry-browsehappy           -308 B  (-0.2%)     -223 B  (-0.4%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~33718 bytes added 📈 [gzipped])

name                                parsed_size             gzip_size
a8c-for-agencies-team                 +534854 B  (+125.5%)  +128125 B  (+96.8%)
a8c-for-agencies-overview             +533131 B  (+108.4%)  +127576 B  (+83.4%)
jetpack-cloud-agency-sites-v2         +461401 B   (+27.7%)  +113341 B  (+24.1%)
a8c-for-agencies-sites                +461371 B   (+26.8%)  +114202 B  (+23.8%)
a8c-for-agencies-client               +398759 B   (+43.8%)   +83721 B  (+30.4%)
a8c-for-agencies-referrals            +394820 B   (+57.6%)   +82311 B  (+38.2%)
github-deployments                    +389724 B   (+35.6%)   +80559 B  (+23.7%)
hosting                               +388766 B   (+26.8%)   +81195 B  (+18.6%)
staging-site                          +388602 B   (+31.9%)   +80479 B  (+21.5%)
sites-dashboard                       +388579 B   (+40.8%)   +80344 B  (+26.8%)
site-monitoring                       +388463 B   (+35.6%)   +80365 B  (+23.1%)
site-logs                             +388463 B   (+35.5%)   +80365 B  (+23.1%)
hosting-features                      +388463 B   (+38.4%)   +80356 B  (+25.3%)
update-design-flow                      -2695 B    (-0.2%)     -958 B   (-0.3%)
link-in-bio-tld-flow                    -2409 B    (-0.1%)     -741 B   (-0.1%)
signup                                  -1880 B    (-0.7%)     -165 B   (-0.3%)
readymade-template-flow                 -1309 B    (-1.0%)    -1086 B   (-3.5%)
with-theme-assembler-flow               -1113 B    (-1.6%)     -139 B   (-1.2%)
update-options-flow                     -1113 B    (-2.2%)     -142 B   (-2.4%)
trial-wooexpress-flow                   -1113 B    (-2.1%)     -142 B   (-2.0%)
tailored-ecommerce-flow                 -1113 B    (-2.1%)     -140 B   (-2.0%)
site-setup-wg                           -1113 B    (-1.3%)     -145 B   (-0.8%)
site-setup-flow                         -1113 B    (-1.3%)     -148 B   (-0.9%)
site-migration-flow                     -1113 B    (-1.8%)     -138 B   (-1.5%)
migration-signup                        -1113 B    (-2.0%)     -133 B   (-1.7%)
migration-flow                          -1113 B    (-1.5%)     -138 B   (-1.0%)
import-flow                             -1113 B    (-1.8%)     -133 B   (-1.4%)
hosted-site-migration-flow              -1113 B    (-1.8%)     -140 B   (-1.5%)
free-post-setup-flow                    -1113 B    (-2.2%)     -139 B   (-2.4%)
free-flow                               -1113 B    (-1.9%)     -145 B   (-1.6%)
entrepreneur-flow                       -1113 B    (-0.9%)     -144 B   (-0.5%)
assembler-first-flow                    -1113 B    (-1.4%)     -141 B   (-1.0%)
ai-assembler-flow                       -1113 B    (-1.4%)     -140 B   (-1.0%)
import                                   +798 B    (+0.1%)      -81 B   (-0.0%)
hundred-year-plan                        -735 B    (-2.1%)     -112 B   (-1.1%)
domains                                  -650 B    (-0.0%)    -2853 B   (-0.5%)
home                                     +643 B    (+0.0%)    +2128 B   (+0.5%)
import-hosted-site-flow                  -608 B    (-0.1%)    -1337 B   (-0.5%)
reader                                   -493 B    (-0.1%)    -1253 B   (-0.4%)
design-first-flow                        -478 B    (-2.0%)      -76 B   (-1.3%)
patterns                                 +477 B    (+0.0%)     -477 B   (-0.1%)
sensei-flow                              -430 B    (-0.1%)    -1083 B   (-0.6%)
settings                                 +395 B    (+0.0%)      +14 B   (+0.0%)
newsletter-flow                          -368 B    (-1.9%)      -96 B   (-1.8%)
link-in-bio-flow                         -368 B    (-2.0%)      -97 B   (-1.9%)
reblogging-flow                          -367 B    (-6.5%)     -122 B   (-6.7%)
onboarding-flow                          -367 B    (-1.2%)     -114 B   (-1.1%)
site-purchases                           +330 B    (+0.0%)     -825 B   (-0.2%)
a8c-for-agencies-settings                +302 B    (+0.2%)     +329 B   (+0.6%)
a8c-for-agencies-plugins                 +302 B    (+0.2%)     +281 B   (+0.6%)
a8c-for-agencies-migrations              +302 B    (+0.2%)     +319 B   (+0.5%)
activity                                 +289 B    (+0.0%)     +468 B   (+0.2%)
videopress-flow                          -275 B    (-0.0%)     -873 B   (-0.3%)
start-writing-flow                       -262 B    (-1.2%)      -64 B   (-1.2%)
a8c-for-agencies-marketplace             -259 B    (-0.0%)     -372 B   (-0.2%)
settings-podcast                         +257 B    (+0.1%)      +69 B   (+0.0%)
media                                    +250 B    (+0.0%)     +646 B   (+0.2%)
plans                                    +246 B    (+0.0%)     +996 B   (+0.2%)
subscribers                              -241 B    (-0.0%)      +99 B   (+0.0%)
jetpack-cloud-plugin-management          -235 B    (-0.0%)      -35 B   (-0.0%)
gutenberg-editor                         +232 B    (+0.0%)      +40 B   (+0.0%)
copy-site-flow                           -217 B    (-0.0%)    -2479 B   (-1.2%)
marketplace                              -207 B    (-0.0%)     -638 B   (-0.2%)
a8c-for-agencies-purchases               +206 B    (+0.0%)     -122 B   (-0.1%)
a8c-for-agencies-landing                 +196 B    (+0.2%)     +415 B   (+1.4%)
jetpack-cloud-pricing                    -193 B    (-0.0%)     -746 B   (-0.3%)
jetpack-cloud-features-comparison        -193 B    (-0.0%)     -842 B   (-0.4%)
stats                                    +177 B    (+0.0%)     -154 B   (-0.0%)
themes                                   -169 B    (-0.0%)     -748 B   (-0.3%)
woocommerce                              +147 B    (+0.0%)      +48 B   (+0.1%)
people                                   +141 B    (+0.0%)       +4 B   (+0.0%)
jetpack-connect                          -141 B    (-0.0%)     -665 B   (-0.2%)
plugins                                  -132 B    (-0.0%)     -166 B   (-0.0%)
backup                                   -113 B    (-0.0%)     +926 B   (+0.3%)
write-flow                               -107 B    (-0.0%)     -288 B   (-0.1%)
build-flow                               -107 B    (-0.0%)     -299 B   (-0.1%)
promote-post-i2                          -100 B    (-0.0%)       +9 B   (+0.0%)
jetpack-cloud-overview                   -100 B    (-0.0%)      -47 B   (-0.0%)
earn                                     -100 B    (-0.0%)     +491 B   (+0.2%)
jetpack-cloud                             +96 B    (+0.0%)     +889 B   (+0.7%)
notification-settings                     -92 B    (-0.0%)      -36 B   (-0.0%)
account                                   -92 B    (-0.0%)      -22 B   (-0.0%)
settings-writing                          -90 B    (-0.0%)     -601 B   (-0.4%)
comments                                  -90 B    (-0.0%)     -749 B   (-0.4%)
switch-site                               +78 B    (+0.0%)      -88 B   (-0.2%)
checkout                                  -77 B    (-0.0%)     -628 B   (-0.1%)
a8c-for-agencies-partner-directory        -77 B    (-0.0%)     -663 B   (-0.5%)
purchases                                 -72 B    (-0.0%)     -414 B   (-0.1%)
settings-security                         -62 B    (-0.0%)      +13 B   (+0.0%)
jetpack-cloud-partner-portal              -61 B    (-0.0%)    -1389 B   (-0.5%)
jetpack-search                            +59 B    (+0.0%)      +96 B   (+0.1%)
theme                                     +54 B    (+0.0%)    +1413 B   (+0.6%)
accept-invite                             +52 B    (+0.0%)      +27 B   (+0.1%)
scan                                      +44 B    (+0.0%)     +299 B   (+0.1%)
settings-performance                      +41 B    (+0.0%)      +34 B   (+0.0%)
posts-custom                              +41 B    (+0.0%)      +31 B   (+0.0%)
posts                                     +41 B    (+0.0%)      +31 B   (+0.0%)
marketing                                 +41 B    (+0.0%)      +34 B   (+0.0%)
devdocs                                   +36 B    (+0.0%)      +33 B   (+0.1%)
jetpack-app                               +25 B    (+0.0%)      -34 B   (-0.0%)
a8c-for-agencies-signup                   +25 B    (+0.0%)      +84 B   (+0.3%)
email                                     +16 B    (+0.0%)     +543 B   (+0.3%)
site-blocks                               +13 B    (+0.0%)      -26 B   (-0.0%)
security                                  +13 B    (+0.0%)     +169 B   (+0.1%)
privacy                                   +13 B    (+0.0%)      -12 B   (-0.0%)
me                                        +13 B    (+0.0%)      -22 B   (-0.0%)
help                                      +13 B    (+0.0%)      -18 B   (-0.0%)
developer                                 +13 B    (+0.0%)      -12 B   (-0.0%)
account-close                             +13 B    (+0.0%)      -19 B   (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~10652 bytes removed 📉 [gzipped])

name                                                                              parsed_size           gzip_size
async-load-automattic-design-preview                                                   +817 B  (+0.0%)     +615 B  (+0.1%)
async-load-design                                                                      -642 B  (-0.0%)     +154 B  (+0.0%)
async-load-signup-steps-domains                                                        -523 B  (-0.1%)    -1810 B  (-1.1%)
async-load-comment-block-editor                                                        +477 B  (+0.0%)     -538 B  (-0.1%)
async-load-automattic-global-styles-src-components-global-styles-variations            +477 B  (+0.0%)     -440 B  (-0.1%)
async-load-signup-steps-theme-selection                                                -457 B  (-0.1%)     -936 B  (-0.8%)
async-load-calypso-post-editor-editor-media-modal                                      +247 B  (+0.0%)     +598 B  (+0.2%)
async-load-calypso-post-editor-media-modal                                             +209 B  (+0.0%)     +569 B  (+0.2%)
async-load-calypso-my-sites-checkout-modal                                             +209 B  (+0.0%)     +953 B  (+0.3%)
async-load-calypso-blocks-editor-checkout-modal                                        +209 B  (+0.0%)    +1026 B  (+0.3%)
async-load-signup-steps-design-picker                                                  +108 B  (+0.2%)      +27 B  (+0.2%)
async-load-signup-steps-site-picker                                                    -105 B  (-0.1%)       -7 B  (-0.0%)
async-load-signup-steps-difm-site-picker                                               -105 B  (-0.1%)       -7 B  (-0.0%)
async-load-calypso-components-sites-popover                                            -105 B  (-0.1%)       -7 B  (-0.0%)
async-load-signup-steps-woocommerce-install-step-store-address                         +103 B  (+0.2%)      +22 B  (+0.1%)
async-load-masterbar-launchpad-navigator                                               -103 B  (-0.2%)      -12 B  (-0.1%)
async-load-automattic-whats-new                                                        -103 B  (-0.5%)      -14 B  (-0.2%)
async-load-calypso-jetpack-cloud-sections-sidebar-navigation-manage-selected-...        +93 B  (+0.1%)     +866 B  (+2.5%)
async-load-calypso-layout-command-palette                                               +78 B  (+0.0%)      -88 B  (-0.2%)
async-load-design-playground                                                            +75 B  (+0.0%)     +104 B  (+0.0%)
async-load-design-wordpress-components-gallery                                          -62 B  (-0.0%)     -168 B  (-0.1%)
async-load-store-app-store-stats-listview                                               +59 B  (+0.0%)      -85 B  (-0.1%)
async-load-store-app-store-stats                                                        +59 B  (+0.0%)      -85 B  (-0.0%)
async-load-signup-steps-user                                                            +52 B  (+0.0%)       +7 B  (+0.0%)
async-load-signup-steps-subscribe-email                                                 +52 B  (+0.0%)      +25 B  (+0.1%)
async-load-design-blocks                                                                -39 B  (-0.0%)     +293 B  (+0.0%)
async-load-calypso-reader-sidebar                                                       +38 B  (+0.0%)      +11 B  (+0.0%)
async-load-calypso-my-sites-stats-summary                                               +38 B  (+0.1%)      +12 B  (+0.1%)
async-load-calypso-my-sites-current-site-notice                                         +38 B  (+0.1%)      +13 B  (+0.1%)
async-load-calypso-blocks-jitm-templates-sidebar-banner                                 +38 B  (+0.1%)      +12 B  (+0.1%)
async-load-calypso-blocks-jitm-templates-notice                                         +38 B  (+0.1%)      +10 B  (+0.1%)
async-load-calypso-blocks-jitm-templates-default                                        +38 B  (+0.1%)      +10 B  (+0.1%)
async-load-signup-steps-plans-theme-preselected                                         +25 B  (+0.0%)      -14 B  (-0.0%)
async-load-signup-steps-plans                                                           +25 B  (+0.0%)      -11 B  (-0.0%)
async-load-automattic-help-center                                                       -23 B  (-0.0%)     -679 B  (-0.4%)
async-load-signup-steps-page-picker                                                     +19 B  (+0.0%)      +23 B  (+0.0%)
async-load-purchase-modal-wrapper                                                       +19 B  (+0.0%)      +21 B  (+0.0%)
async-load-my-sites-checkout-purchase-modal-is-eligible-for-one-click-checkou...        +19 B  (+0.0%)      +24 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@@ -101,7 +103,7 @@ const ItemsDataViews = ( { data, isLoading = false, className }: ItemsDataViewsP
}
onSelectionChange={ data.onSelectionChange }
onChangeView={ data.setDataViewsState }
supportedLayouts={ [ 'table' ] }
defaultLayouts={ { table: {} } }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mayne we should support the array notation upstream as a shortcut.


.dataviews-wrapper {
// Maybe move upstream to the gutenberg repository
width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, it seems the dataviews take 0 width. Maybe this style should be moved directly to the dataviews package in the Gutenberg repository.

@youknowriad youknowriad force-pushed the update/dataviews-package-version branch from 6f4c156 to 4a665f6 Compare August 15, 2024 13:37
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency and predictability, I'd recommend updating @wordpress/dataviews to the version that corresponds to the current monorepo package versions, and then update all packages together in #92571. Let me know if you need any help with that.

package.json Outdated Show resolved Hide resolved
@youknowriad
Copy link
Contributor

For consistency and predictability, I'd recommend updating @wordpress/dataviews to the version that corresponds to the current monorepo package versions, and then update all packages together in #92571. Let me know if you need any help with that.

I agree that that would be better in terms of process but in this case, I think it's fine to make an exception because otherwise we'll do two big migrations and I'd rather do it once into a fully typed package. (And most of the work is already done)

@allilevine
Copy link
Member Author

allilevine commented Aug 16, 2024

I'm documenting Dotcom regressions to work on / make decisions on:

This branch Production
Screen Shot 2024-08-16 at 10 15 06 AM Screen Shot 2024-08-16 at 10 15 13 AM
Screen Shot 2024-08-16 at 10 01 15 AM Screen Shot 2024-08-16 at 10 14 38 AM Screen Shot 2024-08-16 at 10 01 08 AM
Screen Shot 2024-08-16 at 10 32 32 AM Screen Shot 2024-08-16 at 10 32 37 AM
Screen Shot 2024-08-16 at 10 25 59 AM Screen Shot 2024-08-16 at 10 26 08 AM
Screen Shot 2024-08-16 at 10 31 53 AM Screen Shot 2024-08-16 at 10 32 04 AM
Pagination is not fixed Pagination is fixed
Screen Shot 2024-08-16 at 10 05 23 AM Screen Shot 2024-08-16 at 10 05 16 AM
Site column font weight is bold Site column font weight is normal
Screen Shot 2024-08-16 at 10 08 31 AM Screen Shot 2024-08-16 at 10 08 15 AM
Screen Shot 2024-08-16 at 10 12 07 AM Screen Shot 2024-08-16 at 10 11 59 AM
Screen Shot 2024-08-16 at 10 49 38 AM Screen Shot 2024-08-16 at 10 49 44 AM

@allilevine
Copy link
Member Author

On the A4A dashboard, the site error status, Stats column, and Favorites column are missing:

This branch Production
Screen Shot 2024-08-16 at 10 48 10 AM Screen Shot 2024-08-16 at 10 47 52 AM

@allilevine
Copy link
Member Author

@youknowriad I'm struggling to find a futureproof way to hide the Plan column responsively on mobile screen widths (<960px). I tried changing the column width / maxWidth / minWidth to 0px in the view layout, but that didn't hide it. It seems like overkill to hide and show it as the window width changes. Is there a DataViews way to do this?

Screen Shot 2024-08-16 at 2 32 43 PM

@youknowriad
Copy link
Contributor

@allilevine Maybe the best way is to update the fields property of the view prop passed to DataViews dynamically based on useBreakpoint calls.

paddingTop: '2px',
color: '#3858e9 !important',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the DataViews component might be too opinionated with colors, maybe there's a fix to do in Gutenberg here as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great 🙏

@youknowriad
Copy link
Contributor

@allilevine I see that you styled back the search input to how it was in trunk. I agree that it's a better design personally but IMO we should try to be consistent. I don't see why .com's search input should be different from .org's one. Why don't we update the design in .org directly instead?

cc @jameskoster as there might be reasons for the current design in .org.

@jameskoster
Copy link
Contributor

I don't see why .com's search input should be different from .org's one.

I agree.

Iirc the current design was optimised for the Inserter, which was one of the only SearchControl instances at the time. As we implement more instances it may be time to revisit the design. Let's do that in core, keeping mind the ongoing work around the design system.

@allilevine
Copy link
Member Author

@allilevine I see that you styled back the search input to how it was in trunk. I agree that it's a better design personally but IMO we should try to be consistent. I don't see why .com's search input should be different from .org's one. Why don't we update the design in .org directly instead?

Sounds good! I prioritized that regression because it's inconsistent with Domains and Plugins. But Themes is also currently inconsistent, and if our first priority is consistency with core, it makes sense to make the update there. cc'ing @davemart-in @lucasmendes-design on this decision.

@oandregal
Copy link
Contributor

oandregal commented Aug 20, 2024

I've noticed the "Add filter" button styles don't communicate well the disabled state:

core dotcom
Captura de ecrã 2024-08-20, às 08 05 58 Captura de ecrã 2024-08-20, às 08 06 17

I've pinpointed the issue to this dotcom override of the core wp-components. To be fair, it's not the "same color", as an tertiary active button gets the --color-accent while the tertiary disabled button gets the slightly different --color-accent-60. These are too similar to communicate different states, in my view:

Active Disabled
--color-accent: #3858e9 --color-accent-60: #2145e6
#3858e9 #2145e6

@oandregal oandregal force-pushed the update/dataviews-package-version branch from 5dee987 to 9362069 Compare August 20, 2024 08:48
@oandregal
Copy link
Contributor

Pushed a rebase because there were conflicts with trunk.

@youknowriad
Copy link
Contributor

While I don't consider these blockers for this PR (because we hack around them), Here's the list of follow-ups that we should be proposing for the Core package:

One follow-up that I think should be done in calypso instead is the untangling of the "site preview" and the "site title" fields and using primaryField and mediaField configs instead https://github.com/Automattic/wp-calypso/pull/93503/files#r1719527938

Other than that, this PR is in a decent state, we need a bit more testing in order to land it.

@youknowriad
Copy link
Contributor

I've started working on some of the Core follow-ups while this PR is waiting for review/merge.

Here's the first one: Add support for components to the field labels. WordPress/gutenberg#64642

@davemart-in
Copy link
Contributor

Some feedback:

  • Mobile needing a refresh to hide the column is fine.
  • I also think it's fine to keep the pagination at the very bottom of the page.
  • Should we hide the "Add filter" button since it doesn't do anything?
  • When you go to the site management panel, or when you view the dataViews component on mobile, the search box width is just too small. We should manually override that so that it's wider:

CleanShot 2024-08-20 at 07 59 22@2x

  • Can we clean up the alignment here so that the search box vertically aligns with everything else on the left and the cog aligns with the stuff on the right?

CleanShot 2024-08-20 at 08 02 36@2x

@vitozev vitozev requested a review from cleacos August 20, 2024 13:44
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 20, 2024
@davemart-in
Copy link
Contributor

davemart-in commented Sep 3, 2024

By and large, everything looks good! I'm going to call out a few observations. Maybe these are things that need to get fixed upstream. I'm not sure of the severity of these, so I'll just leave the comments and let ya'll sort through them.

  1. The table header row is left aligned in the first column, but not the other columns. This appears to be due to the fact that padding is added to the other table headers as a result of them being buttons:

CleanShot 2024-09-03 at 14 23 15@2x

  1. The action bar row still does not feel vertically aligned with the header or body sections:

CleanShot 2024-09-03 at 14 28 01@2x

  1. The left and right padding here feels excessive on responsive web:

CleanShot 2024-09-03 at 14 37 17@2x

  1. padding-bottom feels like it's 2x that of padding-top. Is there a reason for that? I figured maybe it was to improve scalability, but the downside is that it takes up more vertical space.

CleanShot 2024-09-03 at 14 41 06@2x

  1. None of this seems to persist - If for instance, I click "10" items per page, the current page updates, but if I navigate away from the sites page and then come back, I expect to still see 10 items per page. Instead these settings always seem to reset:

CleanShot 2024-09-03 at 14 33 09@2x

  1. On mobile web, if you click the very top ... menu, the top link in the dropdown (The "Settings") link is hidden behind the masterbar at the top of the page (with no ability to scroll up).

CleanShot 2024-09-03 at 14 44 48@2x

@allilevine
Copy link
Member Author

  1. On mobile web, if you click the very top ... menu, the top link in the dropdown (The "Settings") link is hidden behind the masterbar at the top of the page (with no ability to scroll up).

I added back the table headers on mobile and I think it fixed this as well:

Screen Shot 2024-09-03 at 5 22 06 PM

But I am seeing a flash of scrolling content above the sticky table header:

Screen Shot 2024-09-03 at 5 20 07 PM

@oandregal
Copy link
Contributor

oandregal commented Sep 4, 2024

🟢 Dotcom Sites: it's shippable, in my view.

  • Pushed ede0092 so fields & column width are updated on resizing and not only on mount.
  • @davemart-in the spacing issues you mention seem to be dotcom-specific. Took a quick look but it seems very entangled. With my limited exposure to this code I worry that tweaking something will introduce another issue somewhere. In the interest of shipping this big PR, I'd consider these as follow-ups. For reference, this is how core's dataviews looks (link to storybookwhere people can play with the core component):
Captura de ecrã 2024-09-04, às 08 27 24
  • @davemart-in re: "None of this seems to persist". This is how it also works today in production: if you navigate away from the dataview's page, when coming back the settings are reset to defaults. It's up to the consumer to implement this.

@oandregal
Copy link
Contributor

oandregal commented Sep 4, 2024

🟡 A4A Sites

  • Fix for breakpoints 7600812
  • Remove site notice border-top 7fa6ed9
  • The site notice error remains a challenge. Needs design approval or a fix.
Production Current PR
Captura de ecrã 2024-09-04, às 09 42 56 Captura de ecrã 2024-09-04, às 09 43 15

@oandregal
Copy link
Contributor

🟢 A4A Sites Modal, A4A Referrals (Overview, Details, For Clients), A4A Team (not in production).

@matticbot
Copy link
Contributor

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/dataviews-package-version on your sandbox.

@jameskoster
Copy link
Contributor

jameskoster commented Sep 4, 2024

None of this seems to persist

This is a long-standing issue I'd love to close: WordPress/gutenberg#57669

@allilevine allilevine merged commit cfb9373 into trunk Sep 4, 2024
14 checks passed
@allilevine allilevine deleted the update/dataviews-package-version branch September 4, 2024 16:01
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 4, 2024
allilevine added a commit that referenced this pull request Sep 4, 2024
@youknowriad
Copy link
Contributor

Really cool to see this merged. Thanks all for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.