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

avoid Inventory page components scss overriding other pages #496

Merged
merged 2 commits into from Mar 7, 2021

Conversation

Ron-Lavi
Copy link
Member

@Ron-Lavi Ron-Lavi commented Mar 2, 2021

No description provided.

@Ron-Lavi
Copy link
Member Author

Ron-Lavi commented Mar 2, 2021

@ShimShtein @MariaAga can you check if I missed anything?
looks like our scss overrides other foreman/plugins pages as we load it with deface for 2.3
added a wrapping class in all of the inventory components scss

for insights and the host-details-tab pages looks like they are already explicit,

to review this PR it's best to view it in unified mode and hide spaces so changes are reduced to ~30:
Screenshot (72)

@Ron-Lavi
Copy link
Member Author

Ron-Lavi commented Mar 3, 2021

@adamruzicka that's the one we were talking about,
can you check also if it solves your issue?

Copy link
Member

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

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

Found two more files that need attention:

  • ./webpack/ForemanInventoryUpload/Components/PageHeader/components/SyncButton/components/modal.scss I would rename to something less generic: #inventory_sync_modal for example

  • ./webpack/ForemanInventoryUpload/Components/TabHeader/tabHeader.scss needs wrapping

@MariaAga
Copy link
Member

MariaAga commented Mar 3, 2021

Also:

  • webpack/ForemanInventoryUpload/Components/InventoryFilter/inventoryFilter.scss
  • webpack/ForemanInventoryUpload/Components/InventorySettings/InventorySettings.scss
  • webpack/ForemanInventoryUpload/Components/PageHeader/components/ToolbarButtons/toolbarButtons.scss

Found by searching . with files to include: **/webpack/ForemanInventoryUpload/**/*.scss in vscode

@Ron-Lavi
Copy link
Member Author

Ron-Lavi commented Mar 4, 2021

Also:

  • webpack/ForemanInventoryUpload/Components/InventoryFilter/inventoryFilter.scss
  • webpack/ForemanInventoryUpload/Components/InventorySettings/InventorySettings.scss
  • webpack/ForemanInventoryUpload/Components/PageHeader/components/ToolbarButtons/toolbarButtons.scss

Found by searching . with files to include: **/webpack/ForemanInventoryUpload/**/*.scss in vscode

I think those are actually specific enough as they all start with .inventory-,
from now on I think when we can be more specific in class names/ids that's enough I guess,
I just didn't want to break all of the other pages before the release so I found the solution of wrapping them with the page's class to be the most secure.

@ShimShtein ShimShtein merged commit b7bbe6b into theforeman:master Mar 7, 2021
@ShimShtein
Copy link
Member

Merged, thanks @LaViro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants