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

feat(ui): Make release bubble padding configurable #86841

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+52 −25
Open

feat(ui): Make release bubble padding configurable #86841

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0241175
feat(ui): Make release bubble padding configurable
billyvg Mar 11, 2025
72baff3
fixup calcs
billyvg Mar 11, 2025
7620e90
fix y axis as well
billyvg Mar 11, 2025
e1d037e
fix comments
billyvg Mar 11, 2025
a21eb0f
missed this
billyvg Mar 11, 2025
613d511
move +1 to totalpadding
billyvg Mar 12, 2025
17a62ad
remove constants
billyvg Mar 12, 2025
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
4 changes: 1 addition & 3 deletions static/app/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/constants.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,4 @@ export const DEFAULT_BUBBLE_SIZE = 12;
// "padding" around the bubbles as it is drawn on canvas vs CSS, this may need
// to move into `renderReleaseBubble` if it needs to be customizable
export const RELEASE_BUBBLE_Y_PADDING = 8;
export const RELEASE_BUBBLE_Y_HALF_PADDING = RELEASE_BUBBLE_Y_PADDING / 2;
Copy link
Member Author

Choose a reason for hiding this comment

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

calculate this since padding is now configurable.

export const RELEASE_BUBBLE_X_PADDING = 1;
export const RELEASE_BUBBLE_X_HALF_PADDING = RELEASE_BUBBLE_X_PADDING / 2;
export const RELEASE_BUBBLE_X_PADDING = 2;
39 changes: 29 additions & 10 deletions static/app/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/useReleaseBubbles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ import {
BUBBLE_AREA_SERIES_ID,
BUBBLE_SERIES_ID,
DEFAULT_BUBBLE_SIZE,
RELEASE_BUBBLE_X_HALF_PADDING,
RELEASE_BUBBLE_X_PADDING,
RELEASE_BUBBLE_Y_HALF_PADDING,
RELEASE_BUBBLE_Y_PADDING,
} from 'sentry/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/constants';
import {createReleaseBubbleHighlighter} from 'sentry/views/dashboards/widgets/timeSeriesWidget/releaseBubbles/createReleaseBubbleHighlighter';
Expand Down Expand Up @@ -147,6 +145,7 @@ function createReleaseBubbleMouseListeners({
}

interface ReleaseBubbleSeriesProps {
bubblePadding: {x: number; y: number};
bubbleSize: number;
buckets: Bucket[];
chartRef: React.RefObject<ReactEchartsRef>;
Expand All @@ -165,6 +164,7 @@ function ReleaseBubbleSeries({
chartRef,
theme,
bubbleSize,
bubblePadding,
dateFormatOptions,
}: ReleaseBubbleSeriesProps): CustomSeriesOption | null {
const totalReleases = buckets.reduce((acc, {releases}) => acc + releases.length, 0);
Expand Down Expand Up @@ -192,10 +192,10 @@ function ReleaseBubbleSeries({
* Renders release bubbles underneath the main chart
*/
const renderReleaseBubble: CustomSeriesRenderItem = (
_params: CustomSeriesRenderItemParams,
params: CustomSeriesRenderItemParams,
api: CustomSeriesRenderItemAPI
) => {
const dataItem = data[_params.dataIndex];
const dataItem = data[params.dataIndex];

if (!dataItem) {
return null;
Expand All @@ -221,14 +221,30 @@ function ReleaseBubbleSeries({
// Width between two timestamps for timeSeries
const width = bubbleEndX - bubbleStartX;

// Padding is on both left/right sides to try to center the bubble
//
// bubbleStartX bubbleEndX
// | |
// v v
// ---------------- ----------------
// | | | |
// ---------------- ----------------
// ^ ^
// |--|
// bubblePadding.x
Copy link
Member Author

Choose a reason for hiding this comment

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

note my artisanal ascii art

const shape = {
x: bubbleStartX + RELEASE_BUBBLE_X_HALF_PADDING,
y: bubbleStartY + RELEASE_BUBBLE_Y_HALF_PADDING,
width: width - RELEASE_BUBBLE_X_PADDING,
// currently we have a static height, but this may need to change since
// we have charts of different dimensions
height: bubbleSize - RELEASE_BUBBLE_Y_PADDING,
// No left-padding on first bubble
x: bubbleStartX + (params.dataIndex === 0 ? 0 : bubblePadding.x / 2),
y: bubbleStartY + bubblePadding.y / 2,
// No right-padding on last bubble
// We don't decrease width on first bubble to make up for lack of
// left-padding
width:
width -
(params.dataIndex === 0 || params.dataIndex === data.length - 1
? 0
: bubblePadding.x / 2),
height: bubbleSize - bubblePadding.y,

// border radius
r: 0,
Expand Down Expand Up @@ -294,6 +310,7 @@ ${t('Click to expand')}

interface UseReleaseBubblesParams {
chartRef: React.RefObject<ReactEchartsRef>;
bubblePadding?: {x: number; y: number};
bubbleSize?: number;
chartRenderer?: (rendererProps: {
end: Date;
Expand All @@ -311,6 +328,7 @@ export function useReleaseBubbles({
minTime,
maxTime,
bubbleSize = DEFAULT_BUBBLE_SIZE,
bubblePadding = {x: RELEASE_BUBBLE_X_PADDING, y: RELEASE_BUBBLE_Y_PADDING},
}: UseReleaseBubblesParams) {
const organization = useOrganization();
const {openDrawer} = useDrawer();
Expand Down Expand Up @@ -354,6 +372,7 @@ export function useReleaseBubbles({
releaseBubbleSeries: ReleaseBubbleSeries({
buckets,
bubbleSize,
bubblePadding,
chartRef,
theme,
releases,
Expand Down
Loading