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.

Sign up for GitHub

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

custom dashboard #193

Draft
wants to merge 5 commits into
base: coderabbit_micro_frontend
Choose a base branch
from
Draft

custom dashboard #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e7c30a8
custom dashboard checkpoint
GurinderRawala Nov 29, 2024
a42bf0a
fixes
GurinderRawala Dec 2, 2024
c602459
checkpoint
GurinderRawala Dec 3, 2024
38e481c
checkpoint
GurinderRawala Dec 5, 2024
d57e22e
checkpoint
GurinderRawala Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/grafana-runtime/src/services/LocationService.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface LocationService {
getHistory: () => H.History;
getSearch: () => URLSearchParams;
getSearchObject: () => UrlQueryMap;
fnPathnameChange: (path: string, queryParams: any) => void;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Method signature needs improvements

The new method signature has several issues that should be addressed:

  1. Use a more specific type than any for queryParams (e.g., UrlQueryMap which is already imported)
  2. Consider following the existing method naming patterns (e.g., updatePathname)
  3. Add a return type annotation (likely void)
  4. Add JSDoc documentation like other interface methods
-  fnPathnameChange: (path: string, queryParams: any) => void;
+  /**
+   * Updates the current pathname and query parameters
+   * @param path - The new pathname
+   * @param queryParams - The new query parameters
+   */
+  updatePathname: (path: string, queryParams: UrlQueryMap) => void;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fnPathnameChange: (path: string, queryParams: any) => void;
/**
* Updates the current pathname and query parameters
* @param path - The new pathname
* @param queryParams - The new query parameters
*/
updatePathname: (path: string, queryParams: UrlQueryMap) => void;


/**
* This is from the old LocationSrv interface
Expand Down
1 change: 1 addition & 0 deletions packages/grafana-ui/src/components/Portal/Portal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export function PortalContainer() {
return (
<div
id="grafana-portal-container"
data-qiankun="grafana-full-app"
className={cx({
[styles.grafanaPortalContainer]: isBodyScrolling,
})}
Expand Down
35 changes: 22 additions & 13 deletions public/app/AppWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import { LiveConnectionWarning } from './features/live/LiveConnectionWarning';

interface AppWrapperProps {
app: GrafanaApp;
isMFE?: boolean;
children?: React.ReactNode | null;
}

interface AppWrapperState {
Expand Down Expand Up @@ -88,7 +90,7 @@ export class AppWrapper extends Component<AppWrapperProps, AppWrapperState> {
}

render() {
const { app } = this.props;
const { app, isMFE, children } = this.props;
const { ready } = this.state;

navigationLogger('AppWrapper', false, 'rendering');
Expand Down Expand Up @@ -116,22 +118,29 @@ export class AppWrapper extends Component<AppWrapperProps, AppWrapperState> {
<GlobalStyles />
<div className="grafana-app">
<AppChrome>
<AngularRoot />
<AppNotificationList />
<Stack gap={0} grow={1} direction="column">
{pageBanners.map((Banner, index) => (
<Banner key={index.toString()} />
<>
<AngularRoot />
<AppNotificationList />
<Stack gap={0} grow={1} direction="column">
{pageBanners.map((Banner, index) => (
<Banner key={index.toString()} />
))}
{ready && !isMFE && this.renderRoutes()}
</Stack>
{bodyRenderHooks.map((Hook, index) => (
<Hook key={index.toString()} />
))}
{ready && this.renderRoutes()}
</Stack>
{bodyRenderHooks.map((Hook, index) => (
<Hook key={index.toString()} />
))}
</>
{children}
</AppChrome>
</div>
<LiveConnectionWarning />
<ModalRoot />
<PortalContainer />
{!isMFE && (
<>
<ModalRoot />
<PortalContainer />
</>
)}
</ModalsContextProvider>
</CompatRouter>
</LocationServiceProvider>
Expand Down
32 changes: 23 additions & 9 deletions public/app/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
import { setPanelDataErrorView } from '@grafana/runtime/src/components/PanelDataErrorView';
import { setPanelRenderer } from '@grafana/runtime/src/components/PanelRenderer';
import { setPluginPage } from '@grafana/runtime/src/components/PluginPage';
import config, { updateConfig } from 'app/core/config';
import config, { Settings, updateConfig } from 'app/core/config';
import { arrayMove } from 'app/core/utils/arrayMove';
import { getStandardTransformers } from 'app/features/transformers/standardTransformers';

Expand Down Expand Up @@ -125,15 +125,27 @@ if (process.env.NODE_ENV === 'development') {
export class GrafanaApp {
context!: GrafanaContextType;

async init() {
async init(isMFE = false) {
try {
// Let iframe container know grafana has started loading
parent.postMessage('GrafanaAppInit', '*');

const initI18nPromise = initializeI18n(config.bootData.user.language);
initI18nPromise.then(({ language }) => updateConfig({ language }));

setBackendSrv(backendSrv);
if(isMFE){
backendSrv.setGrafanaPrefix(true);
setBackendSrv(backendSrv);
const settings: Settings = await backendSrv.get('/api/frontend/settings');

config.panels = settings.panels;
config.datasources = settings.datasources;
config.defaultDatasource = settings.defaultDatasource;
} else {
setBackendSrv(backendSrv);
}


initEchoSrv();
initIconCache();
// This needs to be done after the `initEchoSrv` since it is being used under the hood.
Expand Down Expand Up @@ -270,12 +282,14 @@ export class GrafanaApp {

initializeScopes();

const root = createRoot(document.getElementById('reactRoot')!);
root.render(
createElement(AppWrapper, {
app: this,
})
);
if(!isMFE){
const root = createRoot(document.getElementById('reactRoot')!);
root.render(
createElement(AppWrapper, {
app: this,
})
);
}
} catch (error) {
console.error('Failed to start Grafana', error);
window.__grafana_load_failed();
Expand Down
26 changes: 7 additions & 19 deletions public/app/core/components/Page/Page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { css, cx } from '@emotion/css';
import { useLayoutEffect } from 'react';

import { GrafanaTheme2, PageLayoutType } from '@grafana/data';
import { config } from '@grafana/runtime';
import { useStyles2 } from '@grafana/ui';
import { useGrafana } from 'app/core/context/GrafanaContext';

Expand Down Expand Up @@ -94,24 +93,13 @@ Page.Contents = PageContents;

const getStyles = (theme: GrafanaTheme2) => {
return {
wrapper: css(
config.featureToggles.bodyScrolling
? {
label: 'page-wrapper',
display: 'flex',
flex: '1 1 0',
flexDirection: 'column',
position: 'relative',
}
: {
label: 'page-wrapper',
height: '100%',
display: 'flex',
flex: '1 1 0',
flexDirection: 'column',
minHeight: 0,
}
),
wrapper: css({
label: 'page-wrapper',
display: 'flex',
flex: '1 1 0',
flexDirection: 'column',
position: 'relative',
}),
Comment on lines +96 to +102
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the evidence gathered, I can now provide a final response:

Removal of bodyScrolling toggle requires careful consideration

The bodyScrolling feature toggle is extensively used across multiple components in the codebase for controlling scroll behavior and layout calculations. Removing it from the Page component's wrapper styles could lead to inconsistencies since:

  • It's actively used in AppChrome, NativeScrollbar, and other core components
  • It affects global styling through GlobalStyles/page.ts and elements.ts
  • It's integrated with layout calculations in multiple UI components

Please:

  • Keep the feature toggle condition in the wrapper styles to maintain consistency
  • If the goal is to remove this toggle, it should be part of a broader refactoring effort across all affected components
🔗 Analysis chain

Verify removal of bodyScrolling feature toggle

The style implementation has been simplified by removing the conditional logic based on config.featureToggles.bodyScrolling. Please confirm if this standardization of scrolling behavior is intentional and doesn't impact any existing functionality.

The search results show that bodyScrolling feature toggle is used extensively throughout the codebase, affecting scrolling behavior, layout calculations, and styling in multiple components. Let's verify if there are any recent changes or migrations related to this feature toggle.

Let's check if there are any recent changes specifically related to the bodyScrolling feature toggle in the Page component or related files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other usages of bodyScrolling feature toggle
rg "featureToggles.bodyScrolling" -A 3

Length of output: 7304

Script:

#!/bin/bash
# Check recent commits mentioning bodyScrolling
git log --oneline -n 10 | grep -i "bodyscroll"

# Check if there are any migration or cleanup files related to this
fd -e ts -e tsx -e md | rg -i "migration|cleanup|scroll"

Length of output: 4827

Script:

#!/bin/bash
# Check the git history of the Page component and related files
git log -p -- public/app/core/components/Page/Page.tsx | grep -A 5 "bodyScrolling"

# Check for any feature toggle documentation
fd -e md | xargs rg -i "bodyscrolling"

Length of output: 38

pageContent: css({
label: 'page-content',
flexGrow: 1,
Expand Down
Empty file removed public/app/core/context/ModalsProvider.ts
View file
Open in desktop
Empty file.
2 changes: 2 additions & 0 deletions public/app/core/reducers/fn-slice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { AnyObject } from '../../fn-app/types';

export interface FnGlobalState {
FNDashboard: boolean;
isCustomDashboard: boolean;
uid: string;
slug: string;
version: number;
Expand Down Expand Up @@ -48,6 +49,7 @@ export const FN_STATE_KEY = 'fnGlobalState';
export const INITIAL_FN_STATE: FnGlobalState = {
// NOTE: initial value is false
FNDashboard: false,
isCustomDashboard: false,
uid: '',
slug: '',
version: 1,
Expand Down
7 changes: 4 additions & 3 deletions public/app/features/dashboard/components/AddPanelButton/AddPanelButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import AddPanelMenu from './AddPanelMenu';
export interface Props {
dashboard: DashboardModel;
onToolbarAddMenuOpen?: () => void;
isFNDashboard?: boolean;
}

const AddPanelButton = ({ dashboard, onToolbarAddMenuOpen }: Props) => {
const AddPanelButton = ({ dashboard, onToolbarAddMenuOpen, isFNDashboard }: Props) => {
const [isMenuOpen, setIsMenuOpen] = useState(false);

useEffect(() => {
Expand All @@ -29,8 +30,8 @@ const AddPanelButton = ({ dashboard, onToolbarAddMenuOpen }: Props) => {
onVisibleChange={setIsMenuOpen}
>
<Button
variant="secondary"
size="sm"
variant="primary"
size={isFNDashboard ? 'md' : 'sm'}
fill="outline"
data-testid={selectors.components.PageToolbar.itemButton('Add button')}
>
Expand Down
22 changes: 17 additions & 5 deletions public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ const mapStateToProps = (state: StoreState, ownProps: OwnProps) => {
uiState: state.panelEditor.ui,
tableViewEnabled: state.panelEditor.tableViewEnabled,
variables: getVariablesByKey(ownProps.dashboard.uid, state),
isMFEDashboard: state.fnGlobalState.FNDashboard,
isMFECustomDashboard: state.fnGlobalState.isCustomDashboard,
};
};

Expand Down Expand Up @@ -275,6 +277,7 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
dashboard={dashboard}
tabs={tabs}
onChangeTab={this.onChangeTab}
isMfeEditPanel={this.props.isMFEDashboard}
/>
</div>
</SplitPaneWrapper>
Expand Down Expand Up @@ -431,7 +434,8 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
};

render() {
const { initDone, uiState, theme, sectionNav, pageNav, className, updatePanelEditorUIState } = this.props;
const { initDone, uiState, theme, sectionNav, pageNav, className, updatePanelEditorUIState, isMFECustomDashboard } =
this.props;
const styles = getStyles(theme, this.props);

if (!initDone) {
Expand All @@ -444,11 +448,15 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
pageNav={pageNav}
data-testid={selectors.components.PanelEditor.General.content}
layout={PageLayoutType.Custom}
className={className}
className={!isMFECustomDashboard ? className : styles.mfeWrapper}
>
<AppChromeUpdate
actions={<ToolbarButtonRow alignment="right">{this.renderEditorActions()}</ToolbarButtonRow>}
/>
{!isMFECustomDashboard ? (
<AppChromeUpdate
actions={<ToolbarButtonRow alignment="right">{this.renderEditorActions()}</ToolbarButtonRow>}
/>
) : (
<ToolbarButtonRow alignment="right">{this.renderEditorActions()}</ToolbarButtonRow>
)}
<div className={styles.wrapper}>
<div className={styles.verticalSplitPanesWrapper}>
{!uiState.isPanelOptionsVisible ? (
Expand Down Expand Up @@ -495,12 +503,16 @@ export const getStyles = stylesFactory((theme: GrafanaTheme2, props: Props) => {
const paneSpacing = theme.spacing(2);

return {
mfeWrapper: css({
height: '100vh',
}),
wrapper: css({
width: '100%',
flexGrow: 1,
minHeight: 0,
display: 'flex',
paddingTop: theme.spacing(2),
height: '100%',
}),
verticalSplitPanesWrapper: css({
display: 'flex',
Expand Down
9 changes: 5 additions & 4 deletions public/app/features/dashboard/components/PanelEditor/PanelEditorTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ interface PanelEditorTabsProps {
dashboard: DashboardModel;
tabs: PanelEditorTab[];
onChangeTab: (tab: PanelEditorTab) => void;
isMfeEditPanel?: boolean;
}

export const PanelEditorTabs = memo(({ panel, dashboard, tabs, onChangeTab }: PanelEditorTabsProps) => {
export const PanelEditorTabs = memo(({ panel, dashboard, tabs, onChangeTab, isMfeEditPanel }: PanelEditorTabsProps) => {
const forceUpdate = useForceUpdate();
const styles = useStyles2(getStyles);

Expand All @@ -49,7 +50,7 @@ export const PanelEditorTabs = memo(({ panel, dashboard, tabs, onChangeTab }: Pa
return () => eventSubs.unsubscribe();
}, [panel, dashboard, forceUpdate]);

const activeTab = tabs.find((item) => item.active)!;
const activeTab = !isMfeEditPanel? tabs.find((item) => item.active)!: tabs.find((item) => item.id === PanelEditorTabId.Query)!;

if (tabs.length === 0) {
return null;
Expand All @@ -60,7 +61,7 @@ export const PanelEditorTabs = memo(({ panel, dashboard, tabs, onChangeTab }: Pa
return (
<div className={styles.wrapper}>
<TabsBar className={styles.tabBar} hideBorder>
{tabs.map((tab) => {
{!isMfeEditPanel ? tabs.map((tab) => {
if (tab.id === PanelEditorTabId.Alert && alertingEnabled) {
return (
<PanelAlertTab
Expand All @@ -84,7 +85,7 @@ export const PanelEditorTabs = memo(({ panel, dashboard, tabs, onChangeTab }: Pa
counter={getCounter(panel, tab)}
/>
);
})}
}): null}
</TabsBar>
<TabContent className={styles.tabContent}>
{activeTab.id === PanelEditorTabId.Query && <PanelEditorQueries panel={panel} queries={panel.targets} />}
Expand Down
Loading