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

fix: allow custom css stylesheets (CT-000) #80

Merged
merged 2 commits into from
Oct 26, 2023

Conversation

DecathectZero
Copy link
Member

@DecathectZero DecathectZero commented Oct 26, 2023

Users can now initialize stylesheets like this:

window.voiceflow.chat.load({
  ...
  assistant: {
    stylesheet: 'http://test.com/test.css',
  },
});

This leaves the path open to expose this on the GUI in the future since it is part of the Assistant object type.

This is necessary just because of how iframes don't handle root page styles. This stylesheet gets injected both into the main page AND the iframe

@@ -26,5 +26,6 @@ export const sanitizeConfig = (config: unknown): Partial<ChatConfig> & Pick<Chat
...(typeof user.image === 'string' && { image: user.image }),
},
}),
...(isObject(assistant) && ({ assistant } as Partial<Pick<ChatConfig, 'assistant'>>)),
Copy link
Member Author

Choose a reason for hiding this comment

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

we now allow users to add an assistant option the config and override stuff set on VF's webchat settings dashboard.

Comment on lines 10 to 12
link.href = url;

const load = new Promise((resolve, reject) => {
link.onload = resolve;
link.onerror = reject;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is possible, but technically, the resource starts loading right when you set the href attribute. So, in case the resource is already cached, the order of .href and .onload definitions could matter. Again, I don't think that's possible, because of event loop, and even the time it would take to load the cached resource, but better to be safe than sorry.

Suggested change
link.href = url;
const load = new Promise((resolve, reject) => {
link.onload = resolve;
link.onerror = reject;
});
const load = new Promise((resolve, reject) => {
link.onload = resolve;
link.onerror = reject;
});
link.href = url;

@DecathectZero
Copy link
Member Author

bors r+

bors-vf bot added a commit that referenced this pull request Oct 26, 2023
80: fix: allow custom css stylesheets (CT-000) r=DecathectZero a=DecathectZero

Users can now initialize stylesheets like this:
```
window.voiceflow.chat.load({
  ...
  assistant: {
    stylesheet: 'http://test.com/test.css',
  },
});
```

This leaves the path open to expose this on the GUI in the future since it is part of the `Assistant` object type.

This is necessary just because of how iframes don't handle root page styles. This stylesheet gets injected both into the main page AND the iframe


Co-authored-by: Tyler Han <tylerhan97@gmail.com>
@bors-vf
Copy link
Contributor

bors-vf bot commented Oct 26, 2023

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

Response status code: 422
{"message":"This branch must not contain merge commits.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@DecathectZero
Copy link
Member Author

bors r+

bors-vf bot added a commit that referenced this pull request Oct 26, 2023
80: fix: allow custom css stylesheets (CT-000) r=DecathectZero a=DecathectZero

Users can now initialize stylesheets like this:
```
window.voiceflow.chat.load({
  ...
  assistant: {
    stylesheet: 'http://test.com/test.css',
  },
});
```

This leaves the path open to expose this on the GUI in the future since it is part of the `Assistant` object type.

This is necessary just because of how iframes don't handle root page styles. This stylesheet gets injected both into the main page AND the iframe


Co-authored-by: Tyler Han <tylerhan97@gmail.com>
@bors-vf
Copy link
Contributor

bors-vf bot commented Oct 26, 2023

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

Response status code: 422
{"message":"This branch must not contain merge commits. 2 of 2 required status checks are expected.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@DecathectZero
Copy link
Member Author

bors r+

bors-vf bot added a commit that referenced this pull request Oct 26, 2023
80: fix: allow custom css stylesheets (CT-000) r=DecathectZero a=DecathectZero

Users can now initialize stylesheets like this:
```
window.voiceflow.chat.load({
  ...
  assistant: {
    stylesheet: 'http://test.com/test.css',
  },
});
```

This leaves the path open to expose this on the GUI in the future since it is part of the `Assistant` object type.

This is necessary just because of how iframes don't handle root page styles. This stylesheet gets injected both into the main page AND the iframe


Co-authored-by: Tyler Han <tylerhan97@gmail.com>
@bors-vf
Copy link
Contributor

bors-vf bot commented Oct 26, 2023

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

Response status code: 422
{"message":"This branch must not contain merge commits. 2 of 2 required status checks have not succeeded: 1 expected and 1 pending.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@DecathectZero DecathectZero merged commit 7062e0a into master Oct 26, 2023
2 of 4 checks passed
@DecathectZero DecathectZero deleted the tyler/add-css-stylesheet/CT-000 branch October 26, 2023 15:38
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.

3 participants