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

fix(kno-5254): hide inaccessible channels #69

Merged
merged 1 commit into from
Feb 21, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ export const SlackChannelCombobox = ({
channelOptionProps,
inputMessages,
}: Props) => {
const [comboboxListOpen, setComboboxListOpen] = useState<boolean>(false);
const [inputValue, setInputValue] = useState("");

// Used to close the combobox when clicking outside of it
const comboboxRef = useRef(null);
useOutsideClick({ ref: comboboxRef, fn: () => setComboboxListOpen(false) });

// Gather API data
const { connectionStatus, errorLabel: connectionErrorLabel } =
useKnockSlackClient();
Expand All @@ -72,21 +79,25 @@ export const SlackChannelCombobox = ({
updating: connectedChannelsUpdating,
} = useConnectedSlackChannels({ slackChannelsRecipientObject });

const [comboboxListOpen, setComboboxListOpen] = useState<boolean>(false);
const [inputValue, setInputValue] = useState("");

// Used to close the combobox when clicking outside of it
const comboboxRef = useRef(null);
useOutsideClick({ ref: comboboxRef, fn: () => setComboboxListOpen(false) });

// Used to optimistically show when user toggles a channel connected or not
const [currentConnectedChannels, setCurrentConnectedChannels] = useState<
SlackChannelConnection[] | null
>(null);

useEffect(() => {
setCurrentConnectedChannels(connectedChannels);
}, [connectedChannels]);
// Used to make sure we're only showing currently available channels to select from.
// There are cases where a channel is "connected" in Knock, but it wouldn't be
// posting to it if the channel is private and the Slackbot doesn't belong to it,
// so the channel won't show up here and it won't be posted to.
const slackChannelsMap = new Map(
slackChannels.map((channel) => [channel.id, channel]),
);

const channels = connectedChannels?.filter((connectedChannel) => {
return slackChannelsMap.has(connectedChannel.channel_id);
}) || [];

setCurrentConnectedChannels(channels);
}, [connectedChannels, slackChannels]);
Comment on lines 86 to +100
Copy link
Member

Choose a reason for hiding this comment

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

I think this is definitely fine for now since we're pushing to get this out, but semantically might make more sense for this to run in a useCallback? Can worry about it later!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok good call, i will come back to this ty!


const inErrorState = useMemo(
() =>
Expand Down Expand Up @@ -124,26 +135,27 @@ export const SlackChannelCombobox = ({
);
}

const numberConnectedChannels = connectedChannels?.length || 0;
const numberConnectedChannels = currentConnectedChannels?.length || 0;

if (connectedChannels && numberConnectedChannels === 0) {
if (currentConnectedChannels && numberConnectedChannels === 0) {
return (
inputMessages?.noChannelsConnected ||
DEFAULT_INPUT_MESSAGES.noChannelsConnected
);
}

if (connectedChannels && numberConnectedChannels === 1) {
if (currentConnectedChannels && numberConnectedChannels === 1) {
const connectedChannel = slackChannels?.find(
(slackChannel) => slackChannel.id === connectedChannels[0]?.channel_id,
(slackChannel) =>
slackChannel.id === currentConnectedChannels[0]?.channel_id,
);

return (
inputMessages?.singleChannelConnected || `#${connectedChannel?.name}`
inputMessages?.singleChannelConnected || `# ${connectedChannel?.name}`
);
}

if (connectedChannels && numberConnectedChannels > 1) {
if (currentConnectedChannels && numberConnectedChannels > 1) {
return (
inputMessages?.multipleChannelsConnected ||
`${numberConnectedChannels} channels connected`
Expand All @@ -152,35 +164,37 @@ export const SlackChannelCombobox = ({

return "";
}, [
inLoadingState,
connectionStatus,
inLoadingState,
slackChannels,
connectedChannels,
currentConnectedChannels,
inputMessages,
connectionErrorLabel,
]);

// Handle channel click
const handleOptionClick = async (channelId: string) => {
if (!connectedChannels) {
if (!currentConnectedChannels) {
return;
}

const isChannelConnected = connectedChannels.find(
const isChannelConnected = currentConnectedChannels.find(
(channel) => channel.channel_id === channelId,
);

if (isChannelConnected) {
const channelsToSendToKnock = connectedChannels.filter(
const channelsToSendToKnock = currentConnectedChannels.filter(
(connectedChannel) => connectedChannel.channel_id !== channelId,
);

setCurrentConnectedChannels(channelsToSendToKnock);
updateConnectedChannels(channelsToSendToKnock);
} else {
const channelsToSendToKnock = [
...connectedChannels,
...currentConnectedChannels,
{ channel_id: channelId } as SlackChannelConnection,
];

setCurrentConnectedChannels(channelsToSendToKnock);
updateConnectedChannels(channelsToSendToKnock);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ const SlackChannelOption = ({
const [submittedId, setSubmittedId] = useState<string | null>(null);

const icon = () => {
if (submittedId === channel.id && isUpdating) {
return <Spinner size="15px" thickness={3} />;
if (submittedId === channel.id && (isUpdating || isLoading)) {
return <Spinner thickness={3} />;
}

if (isHovered || isConnected) {
return <CheckmarkIcon isConnected={isConnected} />;
}

return <div style={{ width: "15px", height: "18.5px" }} />;
return <div />;
};

const handleOptionClick = (channelId: string) => {
Expand All @@ -51,23 +51,25 @@ const SlackChannelOption = ({
if (submittedId && !isUpdating) {
return setSubmittedId(null);
}
}, [isUpdating, submittedId]);
}, [isLoading, isUpdating, submittedId]);

return (
<button
key={channel.id}
className="rnf-channel-option"
className="rnf-channel-option-button"
onClick={() => !isLoading && handleOptionClick(channel.id)}
disabled={isLoading || isUpdating}
onMouseEnter={() => setIsHovered(true)}
onMouseLeave={() => setIsHovered(false)}
tabIndex={tabIndex}
{...channelOptionProps}
>
<div style={{ marginRight: "0.25rem" }}>{icon()}</div>
<span className="rnf-icon">
{channel.is_private ? <LockIcon /> : <HashtagIcon />}
</span>
<div className="rnf-slack-channel-option-text-with-icon">
<div className="rnf-connected-status-icon">{icon()}</div>
<div className="rnf-icon">
{channel.is_private ? <LockIcon /> : <HashtagIcon />}
</div>
</div>
{channel.name}
</button>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
const CheckmarkIcon = ({ isConnected }: { isConnected: boolean }) => (
const CheckmarkIcon = ({
isConnected,
size = "1rem",
...props
}: {
isConnected: boolean;
size?: string;
}) => (
<svg
width="14"
height="15"
height={size}
width={size}
role="img"
viewBox="0 0 14 15"
fill="none"
xmlns="http://www.w3.org/2000/svg"
{...props}
>
<path
d="M11.3751 3.9996L5.25006 10.9996L2.62506 8.3746"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,26 @@
0px 1px 3px 0px rgba(0, 0, 0, 0.1);
}

.rnf-channel-option {
.rnf-channel-option-button {
display: flex;
padding: 0.5rem 1rem 0.5rem 0.5rem;
padding: 0.5rem 0rem 0.5rem 0rem;
background-color: white;
border-width: 0px;
}

.rnf-channel-option:disabled {
.rnf-channel-option-button:disabled {
color: #3c4257;
cursor: not-allowed;
}

.rnf-icon {
display: flex;
margin-right: 4px;
margin-right: 0.15rem;
margin-top: 0.15rem;
}

.rnf-connected-status-icon {
width: 20px;
}

.rnf-disconnected-info-container {
Expand Down Expand Up @@ -130,3 +135,9 @@
gap: 4px;
align-items: center;
}

.rnf-slack-channel-option-text-with-icon {
display: flex;
gap: 0.25rem;
height: 20px;
}