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

Bug: Fix broken ui components when angular is disabled #78208

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

jackw
Copy link
Contributor

@jackw jackw commented Nov 15, 2023

What is this feature?

I'm not sure if we should apply the change in this PR or wait for individual tasks to remove bootstrap usage from components. e.g #76182 I could only find this one issue so not entirely sure how widespread bootstrap usage is. 🤔

This PR moves the bootstrap import from angular app to react app to resolve issues where certain components break due to missing bootstrap js when angular is disabled. We should consider this an interim solution to buy time for auditing / removing remaining bootstrap usage across the codebase without impacting users that have angular disabled in configs.

Why do we need this feature?

So angular can be disabled without causing ui issues.

Who is this feature for?

Grafana users

Which issue(s) does this PR fix?:

Fixes #76258

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@jackw jackw self-assigned this Nov 15, 2023
@jackw jackw requested review from a team and torkelo as code owners November 15, 2023 14:50
@jackw jackw requested review from ashharrison90, axelavargas and dprokop and removed request for a team November 15, 2023 14:50
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.3.x milestone Nov 15, 2023
Copy link
Member

@axelavargas axelavargas left a comment

Choose a reason for hiding this comment

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

Hey @jackw, thanks for fixing this issue 🥇 . I gave it a test locally, and it resolves the issue reported in these escalations (esc1 and esc 2 ).

My thoughts about this PR are aligned with yours. I think this should be an interim solution. Honestly, I don't know how many other components are relying on bootstrap js in our codebase, and without knowing that, we might be impacting more users who have Angular disabled in their configuration.

P.S. would like to hear your thoughts @dprokop and @torkelo

@jackw jackw added no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Nov 16, 2023
Copy link
Member

@dprokop dprokop left a comment

Choose a reason for hiding this comment

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

@jackw - don't we want to backport this to 10.2.x?

@axelavargas - I don't know neither. Think we need to dedicate some time to learn more about this.

@jackw jackw added backport v10.2.x add to changelog and removed no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Nov 24, 2023
Copy link
Contributor

Hello @jackw!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@jackw
Copy link
Contributor Author

jackw commented Nov 24, 2023

don't we want to backport this to 10.2.x?

Ah well spotted! I've updated the labels for backporting. 👍

@jackw jackw added the type/bug label Nov 24, 2023
@jackw jackw merged commit 1112e90 into main Nov 27, 2023
47 checks passed
@jackw jackw deleted the jackw/fix-bootstrap-broken-components branch November 27, 2023 09:43
grafana-delivery-bot bot pushed a commit that referenced this pull request Nov 27, 2023
fix(frontend): move bootstrap import to app so disabling angular doesnt break ui

(cherry picked from commit 1112e90)
jackw added a commit that referenced this pull request Nov 28, 2023
)

Bug: Fix broken ui components when angular is disabled (#78208)

fix(frontend): move bootstrap import to app so disabling angular doesnt break ui

(cherry picked from commit 1112e90)

Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

When angular support is disable in grafana. Twitter bootstrap JS is not loaded breaking some UI components.
4 participants