8000 feat: integrate backend with idp sync page by jaaydenh · Pull Request #14755 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

feat: integrate backend with idp sync page #14755

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 13 commits into from
Sep 24, 2024
Prev Previous commit
Next Next commit
chore: remove unused
  • Loading branch information
jaaydenh committed Sep 24, 2024
commit 431beea1c078333c261aa5271518dc9c021628d9
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ interface IdpSyncPageViewProps {
export const IdpSyncPageView: FC<IdpSyncPageViewProps> = ({
Copy link
Member

Choose a reason for hiding this comment

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

Have to put the comment here because the code was already in place previously, and it's not selectable for review:

I think we need to add some CSS optical adjustments to the key-value pairs for the IdpField component. Since we're using two different typefaces, their baselines are slightly different. And when you place them right next to each other on the same line, there's enough variation that the text looks sloppy

You might have to open this in a new tab, but the baselines are ever so slightly off, and the cap-heights are, too:
Screenshot 2024-09-23 at 12 21 28 PM

Copy link
Member

Choose a reason for hiding this comment

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

Though a slightly easier and probably much more manageable alternative would be to set all the text in monospace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The monospace font was supposed to be a temporary measure until this page becomes a form and these would become fields. I'll see if I can improve it for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as a temporary fix, I can add some padding to make the baselines match. I really wanted to use the monospace font on only the field value to signify that it is the value of the configuration.

groupSyncSettings,
roleSyncSettings,
groups,
groupsMap,
organization,
}) => {
Expand Down
Loading
0