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

feat: Setup additional node tags #3775

Merged
merged 7 commits into from
Oct 8, 2024
Merged

feat: Setup additional node tags #3775

merged 7 commits into from
Oct 8, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Oct 8, 2024

What does this PR do?

Screen.Recording.2024-10-08.at.13.42.49.mov
Screenshot 2024-10-08 at 14 14 42

Copy link

github-actions bot commented Oct 8, 2024

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr marked this pull request as draft October 8, 2024 12:12
import React from "react";
import { FONT_WEIGHT_SEMI_BOLD } from "theme";

const TooltipWrap = styled(({ className, ...props }: TooltipProps) => (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is repeated a few places now, I'll open up a follow up PR to make a variant or standardise this via the theme.

: theme.palette.text.disabled,
})}
>
{showTags ? <LabelIcon /> : <LabelOffIcon />}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pattern may be restricting us more than we need to by limiting us to using "off" MUI icons if we want to expand this to toggle images or data fields?

https://mui.com/material-ui/material-icons/?query=off

import React from "react";
import { getContrastTextColor } from "styleUtils";
import { FONT_WEIGHT_SEMI_BOLD } from "theme";

export const TAG_DISPLAY_VALUES: Record<
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I have the right colour scheme or setup here @ianjon3s , should be be a quick and easy fix though!

@DafyddLlyr DafyddLlyr requested review from a team and removed request for a team October 8, 2024 13:25
@DafyddLlyr DafyddLlyr requested a review from a team October 8, 2024 13:34
@DafyddLlyr DafyddLlyr marked this pull request as ready for review October 8, 2024 13:34
const showTags = useStore((state) => state.showTags);
if (!showTags) return null;

const tagBgColor = theme.palette.nodeTag[TAG_DISPLAY_VALUES[tag].color];
Copy link
Contributor

Choose a reason for hiding this comment

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

Glad you've extracted this out but it's still quite horrible how nested it is 😂

> = {
placeholder: {
color: "#FAE1B7",
Copy link
Contributor

Choose a reason for hiding this comment

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

Very much approve of the removal of colour hex codes 😁

@DafyddLlyr DafyddLlyr merged commit 854de63 into main Oct 8, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/additional-tags branch October 8, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants