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 a few more SSR problems #35165

Closed
wants to merge 1 commit into from

Conversation

xvaara
Copy link

@xvaara xvaara commented Oct 12, 2021

Tested SSR render with bootstrap-vue-3. Found these.

@xvaara xvaara requested a review from a team as a code owner October 12, 2021 01:49
@GeoSot
Copy link
Member

GeoSot commented Oct 13, 2021

Thank you @xvaara ❤️

@xvaara xvaara requested a review from a team as a code owner October 24, 2021 15:18
@XhmikosR
Copy link
Member

@xvaara please don't include any dist files. Also, try to rebase your branch. If you can't, just let me know and I'll do the rebase (just please make sure you don't overwrite the changes)

@XhmikosR XhmikosR added the skip-changelog So that the release drafter action doesn't include it label Nov 2, 2021
@XhmikosR XhmikosR changed the title fix few ssr problems Fix a few more SSR problems Nov 2, 2021
@XhmikosR XhmikosR removed the request for review from a team November 2, 2021 11:05
@@ -150,7 +150,7 @@ function normalizeParams(originalTypeEvent, handler, delegationFn) {
}

function addHandler(element, originalTypeEvent, handler, delegationFn, oneOff) {
if (typeof originalTypeEvent !== 'string' || !element) {
if (typeof originalTypeEvent !== 'string' || !element || !element.addEventListener) {
Copy link
Member

Choose a reason for hiding this comment

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

@xvaara can you define the reason of this change?

@@ -321,7 +321,7 @@ const getWindow = () => {
* @return {document|{}} The proper element
*/
const getDocument = () => {
return typeof document !== 'undefined' ? document : {}
return typeof document !== 'undefined' ? document : { documentElement: {} }
Copy link
Member

Choose a reason for hiding this comment

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

@xvaara can you define the reason of this change, too?

@GeoSot
Copy link
Member

GeoSot commented Nov 8, 2021

Closing it as not answered. The focusTrap changes transferred to #34989

@GeoSot GeoSot closed this Nov 8, 2021
GeoSot added a commit that referenced this pull request Nov 8, 2021
@xvaara
Copy link
Author

xvaara commented Nov 8, 2021

I did answer... ? @GeoSot are the review comment replies not the right place to answer?

xvaara added a commit to xvaara/bootstrap that referenced this pull request Nov 8, 2021
* upstream/jo-ssr-friendly:
  some updates on focustrap (based on twbs#35165)
  Move ESLint config to an override
  add swipe, add @doc
  use a ternary
  Be SSR friendly when accessing DOM objects
  Fix spacing utility classes mentioned in navbar supported content documentation (twbs#35328)
  README.md remove broken "David DM" dependency badges (twbs#35313)
  Update import stacks required for modifying utilities (twbs#35320)
  Add top placement info to offcanvas docs
@xvaara xvaara deleted the jo-ssr-friendly branch November 8, 2021 17:34
GeoSot added a commit that referenced this pull request Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-reply js skip-changelog So that the release drafter action doesn't include it v5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants