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

Secure z-indexes #5415

Closed
Reinmar opened this issue Oct 3, 2017 · 9 comments
Closed

Secure z-indexes #5415

Reinmar opened this issue Oct 3, 2017 · 9 comments
Labels
package:ui resolution:expired This issue was closed due to lack of feedback. status:stale type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented Oct 3, 2017

Right now sticky panel has z-index:999 and the ctx balloon has also 999. I think that they are correctly displayed (ctx balloon over sticky panel) because they are in the right order in the DOM.

IMO, z-indexes should differ here.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 3, 2017

@oleq
Copy link
Member

oleq commented Oct 3, 2017

I think "Solution 2" in #5352 is the right thing to do.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 3, 2017

Solution 2 is not a solution to this issue (I mean – not directly). Here we should make sure that different UI elements have different z-indexes. So "Solution 2" must be applied together with a rule that different classes are used.

@oleq
Copy link
Member

oleq commented Oct 3, 2017

Yes, that's what I meant.

@Reinmar
Copy link
Member Author

Reinmar commented Jan 24, 2018

Is it done or will be done in this iteration? If not, please move it to it15.

@oleq
Copy link
Member

oleq commented Jan 25, 2018

WDYT @dkonopka? What impact would this have on Letters?

@dkonopka
Copy link
Contributor

dkonopka commented Jan 25, 2018

I'm not so sure because in Letters I've never met similar problem related to same z-index of UI elements. Like in v5, we are using own CSS variables:

:root {
    --ltrs-z-under: -1;
    --ltrs-z-default: 1;
    --ltrs-z-float: 99;
    --ltrs-z-air: 999;
    --ltrs-z-cover: 9999;
    --ltrs-z-overall: 9999999;
}
@define-mixin generate-z-classes $prefix {
	.$prefix {
		z-index: var(--$prefix);
	}
}

IMHO we can manage z-indexes in ckeditor5 with more than two current variables.

:root {
    --ck-z-default: 1;
    --ck-z-modal: 999;
}

Or we can use variables based on UI element

:root {
    --ck-z-sticky-panel: 999;
    --ck-z-balloon-panel: calc( var(--ck-z-sticky-panel) + 1)
}

but it's just an idea.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-ui Oct 9, 2019
@mlewand mlewand added this to the backlog milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. package:ui labels Oct 9, 2019
@pomek pomek removed this from the backlog milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 2, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:ui resolution:expired This issue was closed due to lack of feedback. status:stale type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

6 participants