#9222 Add policy for hiding the sidebar logo #9224

mnholtz · 2024-10-02T21:21:07Z

What does this PR do?

Closes Add Chrome Extension managed storage policy for hiding sidebar logo #9222 and Remove logo in Sidebar for a subset of users #9206
Overrides the PixieBrix sidebar logo visibility specified by the team theme if showSidebarLogo policy is set in managed storage

Demo

https://www.loom.com/share/032d1216eb8d4175a46c15debd8dc00c

github-actions · 2024-10-02T21:38:36Z

Playwright test results

127 passed
7 flaky
4 skipped

Details

Open report ↗︎
138 tests across 45 suites
36 minutes, 31 seconds
cb2355f
For more information on how to debug and view this report, see our readme

Flaky tests

chrome › tests/pageEditor/addStarterBrick.spec.ts › Add starter brick to mod
msedge › tests/pageEditor/addStarterBrick.spec.ts › Add starter brick to mod
chrome › tests/pageEditor/brickActions.spec.ts › brick actions panel behavior
msedge › tests/pageEditor/brickConfiguration.spec.ts › brick configuration
chrome › tests/pageEditor/copyMod.spec.ts › run a copied mod with a built-in integration
msedge › tests/pageEditor/copyMod.spec.ts › run a copied mod with a built-in integration
msedge › tests/pageEditor/logsPane.spec.ts › can view error logs

Skipped tests

chrome › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
msedge › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
msedge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options

codecov · 2024-10-02T21:49:16Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.82%. Comparing base (8318d74) to head (cb2355f).
Report is 334 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9224      +/-   ##
==========================================
+ Coverage   74.24%   74.82%   +0.57%     
==========================================
  Files        1332     1367      +35     
  Lines       40817    42197    +1380     
  Branches     7634     7893     +259     
==========================================
+ Hits        30306    31574    +1268     
- Misses      10511    10623     +112

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…b.com/pixiebrix/pixiebrix-extension into feature/9222_hide_sidebar_logo_policy merge origin

mnholtz · 2024-10-02T22:09:59Z

src/sidebar/Header.tsx

-      {showSidebarLogo && (
-        // `mx-auto` centers the logo
-        <div className="mx-auto">
+      <div className="mx-auto">


Always include a placeholder for the image so that it still fills the space when hidden; before the extension buttons (e.g. the cog button) were left-aligned instead of staying in place

src/store/enterprise/managedStorageTypes.ts

twschiller · 2024-10-03T10:46:40Z

src/hooks/useTheme.ts

+  const { data: cachedTheme, isLoading: isCachedThemeLoading } =
+    useAsyncExternalStore(themeStorageSubscribe, themeStorage.get);
+
+  const { data: hideSidebarLogo, isLoading: isManagedStorageLoading } =


NIT: instead of using useAsyncState with readManagedStorageByKey, we might consider using useManagedStorageState (and standardizing that hook return value use theAsyncState shape)

TIL we have a useManagedStorageState hook. Since we're releasing tomorrow, let's merge this as-is.

@mnholtz before you move on to something else, can you open up a follow-up PR to leverage useManageStorageState, including the enhancement to return AsyncState?

Ticket here: #9227

twschiller · 2024-10-03T10:48:33Z

@Pashaminkovsky @mnholtz reminder that this PR will require our public documentation to be updated

Pashaminkovsky · 2024-10-03T11:19:41Z

@twschiller to update the docs would we add a new section here: https://docs.pixiebrix.com/enterprise-it-setup/browser-extension-installation-and-configuration/browser-extension-configuration-policy/extension-authentication-configuration
Or did you have other docs in mind?

twschiller · 2024-10-03T11:32:49Z

@twschiller to update the docs would we add a new section here: https://docs.pixiebrix.com/enterprise-it-setup/browser-extension-installation-and-configuration/browser-extension-configuration-policy/extension-authentication-configuration Or did you have other docs in mind?

That sub-page is for the extension configuration. It might be the parent page: https://docs.pixiebrix.com/enterprise-it-setup/browser-extension-installation-and-configuration/browser-extension-configuration-policy

Although currently we just point people to the JSON Schema there:

The custom branding page might be a place to mention it and link to the extension configuration page: https://docs.pixiebrix.com/enterprise-it-setup/custom-branding-and-themes. (The custom branding page is also slightly out of date, e.g., it doesn't mention changing the toolbar icon logo)

static/managedStorageSchema.json

grahamlangford · 2024-10-03T13:57:52Z

src/store/enterprise/managedStorageTypes.ts

+   * Hides the logo in the PixieBrix sidebar, overriding the team theme.
+   * @since 2.1.4
+   */
+  hideSidebarLogo?: boolean;


Should this be:

Suggested change

hideSidebarLogo?: boolean;

hideSidebarLogo: boolean;

Since the schema says it defaults to false? Or does that default not actually do anything?

That's a great question. Upon manual inspection the default value doesn't seem to do anything. I also wasn't able to find mention of this value in the docs anywhere https://developer.chrome.com/docs/extensions/reference/manifest/storage

That's a great question. Upon manual inspection the default value doesn't seem to do anything. I also wasn't able to find mention of this value in the docs anywhere

That's correct - the default does not flow through to Chrome. In general, default in JSON Schemas don't have runtime implications. It's a hint to tools. See: https://json-schema.org/understanding-json-schema/reference/annotations#annotations

The default keyword specifies a default value. This value is not used to fill in missing values during the validation process. Non-validation tools such as documentation generators or form generators may use this value to give hints to users about how to use a value. However, default is typically used to express that if a value is missing, then the value is semantically the same as if the value was present with the default value. The value of default should validate against the schema in which it resides, but that isn't required.

mnholtz · 2024-10-03T16:41:17Z

@Pashaminkovsky FYI, for documentation purposes the new policy name is showSidebarLogo and will override the team theme if set in both cases of being true or false

grahamlangford · 2024-10-03T19:09:39Z

src/hooks/useTheme.test.ts

    expect(activateTheme).toHaveBeenCalledOnce();
  });
+
+  it.each([{ policyValue: true }, { policyValue: false }])(


For completeness, we should update this to be something like:

[ { policyValue: true, themeValue: true, expectedValue: true }, { policyValue: true, themeValue: false, expectedValue: true }, { policyValue: false, themeValue: true, expectedValue: false }, { policyValue: false, themeValue: false, expectedValue: false }, { policyValue: undefined, themeValue: true, expectedValue: true }, { policyValue: undefined, themeValue: false, expectedValue: false } ]

grahamlangford

LGTM. See comment about improving the tests

github-actions · 2024-10-03T19:16:07Z

When the PR is merged, the first loom link found on this PR will be posted to #sprint-demo on Slack. Do not edit this comment manually.

mnholtz added 2 commits October 2, 2024 13:17

add hideSidebarLogo to manatgedStorageSchema

0ee3c37

override showSidebarLogo in useTheme

2bcc02e

mnholtz requested review from grahamlangford and fungairino October 2, 2024 21:21

add unit tests to useTheme

5d60f92

vercel bot deployed to Preview October 2, 2024 21:42 View deployment

mnholtz marked this pull request as ready for review October 2, 2024 22:00

Merge branch 'main' into feature/9222_hide_sidebar_logo_policy

d3db8e9

vercel bot deployed to Preview October 2, 2024 22:01 View deployment

mnholtz added 2 commits October 2, 2024 15:06

fix header button alignment when missing sidebar logo

7b1e90e

Merge branch 'feature/9222_hide_sidebar_logo_policy' of https://githu…

319fdb6

…b.com/pixiebrix/pixiebrix-extension into feature/9222_hide_sidebar_logo_policy merge origin

vercel bot deployed to Preview October 2, 2024 22:08 View deployment

mnholtz commented Oct 2, 2024

View reviewed changes

src/store/enterprise/managedStorageTypes.ts Outdated Show resolved Hide resolved

twschiller added the enhancement New feature or request label Oct 3, 2024

twschiller reviewed Oct 3, 2024

View reviewed changes

twschiller assigned mnholtz Oct 3, 2024

twschiller requested a review from Pashaminkovsky October 3, 2024 10:47

grahamlangford reviewed Oct 3, 2024

View reviewed changes

static/managedStorageSchema.json Outdated Show resolved Hide resolved

grahamlangford reviewed Oct 3, 2024

View reviewed changes

mnholtz added 2 commits October 3, 2024 09:00

refactor rename hideSidebarLogo -> showSidebarLogo

afa7f80

refactor combine test cases

58f65a8

vercel bot deployed to Preview October 3, 2024 16:13 View deployment

mnholtz added 2 commits October 3, 2024 09:31

add add custom theme to test cases

3924d4d

deck version number in code comment

ebba63f

vercel bot deployed to Preview October 3, 2024 16:34 View deployment

mnholtz mentioned this pull request Oct 3, 2024

Refactor useTheme to use useManagedStorageState #9227

Open

Merge branch 'main' into feature/9222_hide_sidebar_logo_policy

4cba229

vercel bot deployed to Preview October 3, 2024 16:45 View deployment

mnholtz linked an issue Oct 3, 2024 that may be closed by this pull request

Remove logo in Sidebar for a subset of users #9206

Closed

grahamlangford reviewed Oct 3, 2024

View reviewed changes

grahamlangford approved these changes Oct 3, 2024

View reviewed changes

make test cases complete

cb2355f

vercel bot deployed to Preview October 3, 2024 19:38 View deployment

mnholtz enabled auto-merge (squash) October 3, 2024 19:38

mnholtz merged commit 41b948a into main Oct 3, 2024
20 checks passed

mnholtz deleted the feature/9222_hide_sidebar_logo_policy branch October 3, 2024 20:17

mnholtz added this to the 2.1.3 milestone Oct 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#9222 Add policy for hiding the sidebar logo #9224

#9222 Add policy for hiding the sidebar logo #9224

mnholtz commented Oct 2, 2024 •

edited

Loading

github-actions bot commented Oct 2, 2024 •

edited

Loading

codecov bot commented Oct 2, 2024 •

edited

Loading

mnholtz Oct 2, 2024 •

edited

Loading

twschiller Oct 3, 2024 •

edited

Loading

grahamlangford Oct 3, 2024

mnholtz Oct 3, 2024

twschiller commented Oct 3, 2024

Pashaminkovsky commented Oct 3, 2024

twschiller commented Oct 3, 2024 •

edited

Loading

grahamlangford Oct 3, 2024

mnholtz Oct 3, 2024

twschiller Oct 3, 2024 •

edited

Loading

mnholtz commented Oct 3, 2024

grahamlangford Oct 3, 2024

grahamlangford left a comment

github-actions bot commented Oct 3, 2024

#9222 Add policy for hiding the sidebar logo #9224

#9222 Add policy for hiding the sidebar logo #9224

Conversation

mnholtz commented Oct 2, 2024 • edited Loading

What does this PR do?

Demo

github-actions bot commented Oct 2, 2024 • edited Loading

Playwright test results

Details

codecov bot commented Oct 2, 2024 • edited Loading

Codecov Report

mnholtz Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

twschiller Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

grahamlangford Oct 3, 2024

Choose a reason for hiding this comment

mnholtz Oct 3, 2024

Choose a reason for hiding this comment

twschiller commented Oct 3, 2024

Pashaminkovsky commented Oct 3, 2024

twschiller commented Oct 3, 2024 • edited Loading

grahamlangford Oct 3, 2024

Choose a reason for hiding this comment

mnholtz Oct 3, 2024

Choose a reason for hiding this comment

twschiller Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

mnholtz commented Oct 3, 2024

grahamlangford Oct 3, 2024

Choose a reason for hiding this comment

grahamlangford left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 3, 2024

mnholtz commented Oct 2, 2024 •

edited

Loading

github-actions bot commented Oct 2, 2024 •

edited

Loading

codecov bot commented Oct 2, 2024 •

edited

Loading

mnholtz Oct 2, 2024 •

edited

Loading

twschiller Oct 3, 2024 •

edited

Loading

twschiller commented Oct 3, 2024 •

edited

Loading

twschiller Oct 3, 2024 •

edited

Loading