Skip to content

feat(issue-views): Improve drag handle and safari interactions #87119

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

Merged
merged 2 commits into from
Mar 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 19 additions & 14 deletions static/app/components/nav/issueViews/issueViewNavItemContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {motion, Reorder, useDragControls} from 'framer-motion';
import type {Location} from 'history';
import isEqual from 'lodash/isEqual';

import InteractionStateLayer from 'sentry/components/interactionStateLayer';
import {useNavContext} from 'sentry/components/nav/context';
import {GrabHandleIcon} from 'sentry/components/nav/issueViews/grabHandleIcon';
import IssueViewNavEditableTitle from 'sentry/components/nav/issueViews/issueViewNavEditableTitle';
Expand Down Expand Up @@ -38,7 +39,7 @@ export interface IssueViewNavItemContentProps {
/**
* Whether an item is being dragged.
*/
isDragging: boolean;
isDragging: string | null;
/**
* Whether the item is the last view in the list.
* This will be removed once view sharing/starring is implemented.
Expand All @@ -63,7 +64,7 @@ export interface IssueViewNavItemContentProps {
/**
* A callback function that updates the isDragging state.
*/
setIsDragging: (isDragging: boolean) => void;
setIsDragging: (isDragging: string | null) => void;
/**
* The issue view to display
*/
Expand Down Expand Up @@ -137,11 +138,11 @@ export function IssueViewNavItemContent({
dragTransition={{bounceStiffness: 400, bounceDamping: 40}}
value={view}
onDragStart={() => {
setIsDragging(true);
setIsDragging(view.id);
startInteraction();
}}
onDragEnd={() => {
setIsDragging(false);
setIsDragging(null);
onReorderComplete();
endInteraction();
}}
Expand All @@ -168,6 +169,7 @@ export function IssueViewNavItemContent({
e.preventDefault();
}}
>
<StyledInteractionStateLayer isPressed={isDragging === view.id} />
<GrabHandleIcon color="gray300" />
</GrabHandleWrapper>
<ProjectIcon projectPlatforms={projectPlatforms} />
Expand Down Expand Up @@ -218,7 +220,7 @@ export function IssueViewNavItemContent({
});
}}
setIsEditing={setIsEditing}
isDragging={isDragging}
isDragging={!!isDragging}
/>
{view.unsavedChanges && (
<Tooltip
Expand Down Expand Up @@ -344,7 +346,13 @@ const hasUnsavedChanges = (
// but we need to ensure the item is relatively positioned and has a background color for it to work
const StyledReorderItem = styled(Reorder.Item)`
position: relative;
background-color: ${p => p.theme.surface200};
background-color: ${p => p.theme.translucentSurface200};
border-radius: ${p => p.theme.borderRadius};
`;

const StyledInteractionStateLayer = styled(InteractionStateLayer)`
height: 120%;
border-radius: 4px;
`;

const TrailingItemsWrapper = styled('div')`
Expand All @@ -357,14 +365,12 @@ const StyledSecondaryNavItem = styled(SecondaryNav.Item)`
position: relative;
padding-right: ${space(0.5)};

/* Hide the ellipsis menu if not hovered, or if it's not expanded */
:not(:hover):not(:has([data-ellipsis-menu-trigger][aria-expanded='true'])) {
[data-ellipsis-menu-trigger] {
/* Hide the ellipsis menu if the item is not hovered */
:not(:hover) {
[data-ellipsis-menu-trigger]:not([aria-expanded='true']) {
${p => p.theme.visuallyHidden}
}
}

:not(:hover) {
[data-drag-icon] {
${p => p.theme.visuallyHidden}
}
Expand All @@ -381,8 +387,7 @@ const StyledSecondaryNavItem = styled(SecondaryNav.Item)`
}

/* Hide the query count if the ellipsis menu is expanded */
&:has([data-ellipsis-menu-trigger][aria-expanded='true'])
[data-issue-view-query-count] {
:has([data-ellipsis-menu-trigger][aria-expanded='true']) [data-issue-view-query-count] {
${p => p.theme.visuallyHidden}
}
`;
Expand Down Expand Up @@ -413,13 +418,13 @@ const LeadingItemsWrapper = styled('div')`
display: flex;
align-items: center;
justify-content: center;
margin-right: ${space(0.75)};
`;

const GrabHandleWrapper = styled(motion.div)`
display: flex;
align-items: center;
justify-content: center;
margin-right: ${space(0.75)};
width: 18px;
height: 18px;
cursor: grab;
Expand Down
2 changes: 1 addition & 1 deletion static/app/components/nav/issueViews/issueViewNavItems.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function IssueViewNavItems({
* source of truth for the view state, rather than a separate state
*/
const [views, setViews] = useState<IssueView[]>(loadedViews);
const [isDragging, setIsDragging] = useState(false);
const [isDragging, setIsDragging] = useState<string | null>(null);
const queryClient = useQueryClient();

useEffect(() => {
Expand Down
11 changes: 7 additions & 4 deletions static/app/components/nav/projectIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import styled from '@emotion/styled';
import PlatformIcon from 'platformicons/build/platformIcon';

import {IconAllProjects} from 'sentry/components/nav/iconAllProjects';
import {space} from 'sentry/styles/space';

interface ProjectIconProps {
projectPlatforms: string[];
className?: string;
}

function ProjectIcon({projectPlatforms}: ProjectIconProps) {
function ProjectIcon({projectPlatforms, className}: ProjectIconProps) {
let renderedIcons: React.ReactNode;

switch (projectPlatforms.length) {
Expand Down Expand Up @@ -36,7 +36,11 @@ function ProjectIcon({projectPlatforms}: ProjectIconProps) {
);
}

return <IconWrap data-project-icon>{renderedIcons}</IconWrap>;
return (
<IconWrap className={className} data-project-icon>
{renderedIcons}
</IconWrap>
);
}

const IconWrap = styled('div')`
Expand All @@ -45,7 +49,6 @@ const IconWrap = styled('div')`
flex-direction: column;
align-items: center;
justify-content: center;
margin-right: ${space(0.75)};
Copy link
Member Author

Choose a reason for hiding this comment

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

The project icon should not have an opinion on margin

`;

const IconContainer = styled('div')`
Expand Down
8 changes: 7 additions & 1 deletion static/app/views/insights/navigation.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import {Fragment} from 'react';
import styled from '@emotion/styled';

import {NAV_GROUP_LABELS} from 'sentry/components/nav/constants';
import {usePrefersStackedNav} from 'sentry/components/nav/prefersStackedNav';
import ProjectIcon from 'sentry/components/nav/projectIcon';
import {SecondaryNav} from 'sentry/components/nav/secondary';
import {PrimaryNavGroup} from 'sentry/components/nav/types';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import useOrganization from 'sentry/utils/useOrganization';
import useProjects from 'sentry/utils/useProjects';
import {
Expand Down Expand Up @@ -70,7 +72,7 @@ function InsightsSecondaryNav({children}: InsightsNavigationProps) {
key={project.id}
to={`${baseUrl}/projects/${project.slug}/`}
leadingItems={
<ProjectIcon
<StyledProjectIcon
projectPlatforms={project.platform ? [project.platform] : []}
/>
}
Expand Down Expand Up @@ -98,3 +100,7 @@ export default function InsightsNavigation({children}: InsightsNavigationProps)

return <InsightsSecondaryNav>{children}</InsightsSecondaryNav>;
}

const StyledProjectIcon = styled(ProjectIcon)`
margin-right: ${space(0.75)};
`;
Loading