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.

Sign up for GitHub

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

Jump to bottom

/patterns/* live examples error #10400

Closed
yurm04 opened this issue Sep 11, 2023 · 2 comments
Closed

/patterns/* live examples error #10400

yurm04 opened this issue Sep 11, 2023 · 2 comments
Assignees
@gwyneplaine
Labels
Engineering

Comments

@yurm04
Copy link
Contributor

yurm04 commented Sep 11, 2023 •
edited
Loading

Two issues with the pattern live examples on a patterns pages:

  1. When you first load the website locally using yarn run dev and navigate to /patterns/app-settings-layout there is an error loaded in the example. This only happens of the first load and then by navigating to /patterns => /patterns/app-settings-layout
    Image

  2. The patterns example Show code button does not work when clicked

Screen.Recording.2023-09-11.at.2.51.15.PM.mov
@yurm04 yurm04 changed the title /patterns/* examples error /patterns/* live examples error Sep 11, 2023
@gwyneplaine
Copy link
Contributor

@yurm04 first issue only happens locally due to us having two servers:

  1. next server running the docs site
  2. a webpack dev server running the playroom instance

This won't happen in prod or staging, as they will all be prebuilt artefacts. Worthwhile noting that this was happening before the uplift docs updates. That said happy to look into it a bit more if its something you feel strongly about but it may be significant effort 👍

Looking into the second issue now 👍

@gwyneplaine gwyneplaine self-assigned this Sep 14, 2023
@gwyneplaine gwyneplaine mentioned this issue Sep 14, 2023
Merged
5 tasks
gwyneplaine added a commit that referenced this issue Sep 15, 2023
@gwyneplaine
Docs update CodeVisibilityProvider to properly inherit from parent …
4e884ee 
…context (#10467)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #10400  <!-- link to issue if one exists -->

### WHAT is this pull request doing?

Change the type signature of CodeVisibilityContext values 
```diff
- [bool, (arg: bool) => void]
+ { showCode?: bool, setShowCode?: (arg: bool) => void }
```
This is to ensure that we don't end up always inheriting the default
values set by our createContext invocation for `CodeVisibilityContext`

Changed the value instantiation for the CodeVisiblityProvider in
Markdown.tsx
```diff
-[
- codeVisibleFromContext ?? showCode,
- setShowCodeFromContext ?? setShowCode
-]
+{
+     showCode: typeof codeVisibleFromContext !== undefined ?? showCode,
+       setShowCode: setShowCodeFromContext ?? setShowCode,
+}
```
To ensure that we also respect and inherit explicit `false` values when
set in the parent context.


### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
@jesstelford
Copy link
Contributor

To add some more detail to issue #1; The local webpack server @gwyneplaine mentioned takes longer to boot up, which is why you're only seeing it when you first load the page - the local Playroom server is still starting. If you watch the terminal output, you'll see it starts about 5-10s after Next does (something like Listening on port 9000), then the embedded playroom examples will work without issue 👍

sophschneider pushed a commit that referenced this issue Sep 19, 2023
@gwyneplaine @sophschneider
Docs update CodeVisibilityProvider to properly inherit from parent …
7391444 
…context (#10467)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #10400  <!-- link to issue if one exists -->

### WHAT is this pull request doing?

Change the type signature of CodeVisibilityContext values 
```diff
- [bool, (arg: bool) => void]
+ { showCode?: bool, setShowCode?: (arg: bool) => void }
```
This is to ensure that we don't end up always inheriting the default
values set by our createContext invocation for `CodeVisibilityContext`

Changed the value instantiation for the CodeVisiblityProvider in
Markdown.tsx
```diff
-[
- codeVisibleFromContext ?? showCode,
- setShowCodeFromContext ?? setShowCode
-]
+{
+     showCode: typeof codeVisibleFromContext !== undefined ?? showCode,
+       setShowCode: setShowCodeFromContext ?? setShowCode,
+}
```
To ensure that we also respect and inherit explicit `false` values when
set in the parent context.


### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
@yurm04 yurm04 mentioned this issue Sep 21, 2023
Closed
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this issue Apr 22, 2024
@gwyneplaine
Docs update CodeVisibilityProvider to properly inherit from parent …
6a7f1f1 
…context (Shopify#10467)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes Shopify#10400  <!-- link to issue if one exists -->

### WHAT is this pull request doing?

Change the type signature of CodeVisibilityContext values 
```diff
- [bool, (arg: bool) => void]
+ { showCode?: bool, setShowCode?: (arg: bool) => void }
```
This is to ensure that we don't end up always inheriting the default
values set by our createContext invocation for `CodeVisibilityContext`

Changed the value instantiation for the CodeVisiblityProvider in
Markdown.tsx
```diff
-[
- codeVisibleFromContext ?? showCode,
- setShowCodeFromContext ?? setShowCode
-]
+{
+     showCode: typeof codeVisibleFromContext !== undefined ?? showCode,
+       setShowCode: setShowCodeFromContext ?? setShowCode,
+}
```
To ensure that we also respect and inherit explicit `false` values when
set in the parent context.


### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gwyneplaine gwyneplaine

Labels
Engineering
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jesstelford @yurm04 @gwyneplaine