Maniphest T191993

Fullscreen button broken with newer version of bootstrap-table
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Lucas_Werkmeister_WMDE
	Apr 11 2018, 3:48 PM

Description

The bootstrap-table folks added the following rule to bootstrap-table.css:

.fullscreen {
    position: fixed;
    top: 0;
    left: 0;
    z-index: 1050;
    width: 100%!important;
    background: #FFF;
}

It’s not scoped or anything, so it breaks our fullscreen button – the position: fixed yanks it out of the query helper toolbar:

Screen Shot 2018-04-11 at 17.45.35.png (500×1 px, 24 KB)

For now we can exclude that bootstrap-table version in package.json, but how do we proceed after that? File a bug report against bootstrap-table? (I would be surprised if we’re the only people whose website this breaks…)

Related Objects

Mentioned In: T191952: 404 errors on new WDQS container for style.css

Event Timeline

Lucas_Werkmeister_WMDE created this task.Apr 11 2018, 3:48 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 11 2018, 3:48 PM

Addshore awarded a token.Apr 11 2018, 3:48 PM

I think I fixed it in https://gerrit.wikimedia.org/r/c/425099/ but it may not be the correct fix. It fixes the issue for me.

Yeah, I just saw that… I assume it works (I just hadn’t pulled that commit yet), but I don’t like it: it might still break at any time if another library decides that it wants to restyle fullscreen-toggle as well. So I’m still not sure what to do now, to be honest.

We could switch to doing it by ID, not by class, and use reasonably prefixed IDs. Shouldn't that fix the issue with libraries stepping on our toes with their classes? Or just add wdqs- prefix to our classes - I don't think any library would use that.
My fix was more stopgap measure to not break production GUI. Please feel free to improve it.

Well, quite frankly I think it’s the library authors who should use prefixes for their classes – it shouldn’t be our job to tiptoe around them any time we upgrade our dependencies.

I’ve submitted a pull request for bootstrap-table, let’s see what happens.

Smalyshev mentioned this in T191952: 404 errors on new WDQS container for style.css.Apr 11 2018, 5:09 PM

Rammanojpotla merged a task: T189471: Fullscreen toggle button displayed in wrong place.Apr 11 2018, 5:42 PM

Rammanojpotla subscribed.

Yeah generally it's a Bad Thing (TM) to hog something as generic as class name fullscreen to one library. But we may as well have additional line of defense...
In any case it seems like master is fine for now, but if bootstrap updates we may want to bump the version.

I think fullscreen-toggle makes sense anyways, so I don’t think we need to revert your change or anything.

The pull request was merged, and I don’t think there’s anything else we can do here… let’s hope this doesn’t happen too often in the future.

	F16917255: Screen Shot 2018-04-11 at 17.45.35.png
	Apr 11 2018, 3:48 PM

Fullscreen button broken with newer version of bootstrap-tableClosed, ResolvedPublicActions

Description

Related Objects

Event Timeline

Fullscreen button broken with newer version of bootstrap-table
Closed, ResolvedPublic
Actions