aframevr / aframe

:a: Web framework for building virtual reality experiences.
https://aframe.io/
MIT License
16.69k stars 3.98k forks source link

Prefix CSS Classes for inspector #5422

Open cmahnke opened 10 months ago

cmahnke commented 10 months ago

Description:

Currently The A-Frame Inspector uses quite generic Class names like static and content and certainly others as well, sine these are likely also used by page authors, please consider prefixing them.

dmarcos commented 10 months ago

CC @vincentfretin

vincentfretin commented 10 months ago

The aframe-inspector is using stylus for the style, style is in https://github.com/aframevr/aframe-inspector/tree/master/src/style Most of the css is using simple css rules with just a class name. Some with specificity like #scenegraph .component that will prevent bleeding to your page if you have some elements with class="component" for example. What we saw is also the contrary, additional css in your project may impact the inspector, like this naf issue with class button and I also did some fixes related to tailwind or bootstrap reset styles in the page impacting the inspector.

Adding a prefix to all used class is one possible fix for both issues, this requires changes in the react code to replace the className attribute where the class is used. Here is an exhaustive list of the class names used:

addComponent
assets
button
close
collapse-button
collapsespace
collapsible-content
collapsible-header
color
color_value
color-widget
commonComponents
componentHeader
componentHeaderActions
components
componentTitle
detail
entityActions
entityChildPadding
entityCloseTag
entityIcons
entityName
entityPrint
entityTagName
gallery
gltfIcon
help-key
help-key-def
help-key-unit
help-list
help-lists
hidden
hide
imageUrl
left
local-transform
map_value
mixinOptions
mixinValue
modal
modal-body
modal-content
modal-header
new_asset_options
newimage
number
option
outliner
preview
propertyRow
right
scenegraph
scenegraph-toolbar
search
select-widget
static
string
text
texture
title
toggle-edit
toggle-sidebar
toolbarActions
toolbarButtons
vec2
vec3
vec4

Potentially any of the above class may be an issue. Going through the aframe-inspector code base and renaming all that can be a bit of work. Another solution would be using css modules instead of stylus, that way the class names will be generated and be unique.

Just curious, what class had you issue with?

dmarcos commented 10 months ago

Thanks for the break down

cmahnke commented 10 months ago

@vincentfretin: Thanks, my "frame" uses, or better: used until now (global) static, content and a few more from the above, where content had the most visible (making the inspector nearly unusable) impact.

I was able to change my theme, but I suspect that not everyone can do this, especially when using a CMS or SSG...

dmarcos commented 10 months ago

Maybe worth apply inspector classes to just the children of a parent with the class .aframe-inspector so they don't leak outside? @vincentfretin knows that code better than me so might not be appropriate.

cmahnke commented 10 months ago

@dmarcos: Issues also arise when the surrouding website (or CMS theme or whatever) provides a global .content stylesheet...

There's also the option of running A-Frame inside an iframe to prevent collisions.

dmarcos commented 10 months ago

What should we do here?

cmahnke commented 10 months ago

@dmarcos: From what I've read about the possible options, which are:

I opt for using a React based CSS module approach, since this will take care of the naming.

vincentfretin commented 10 months ago

Yes, like @cmahnke said. For CSS Modules, I was talking about just importing css file and using webpack css-loader https://webpack.js.org/loaders/css-loader/ so nothing to do with React really or styled-components that is also a different thing.

cmahnke commented 10 months ago

Thanks for the clarification @vincentfretin I wasn't aware of this distinction / possibility. Then as you propose.

dmarcos commented 10 months ago

Thanks! So nothing to do on the A-Frame side? Can we close?

cmahnke commented 10 months ago

@dmarcos: Well, it was my understanding that this is a change which is required on the inspector side...

vincentfretin commented 10 months ago

It's a change in aframe-inspector, probably a day or two of work to changes the style code. You can transfer the issue to the aframe-inspector repo to keep it there if you want.

vincentfretin commented 9 months ago

I'm working on changing the way we use fontawesome icons first https://github.com/aframevr/aframe-inspector/pull/706 before starting any migration to css modules.