-
Notifications
You must be signed in to change notification settings - Fork 14
Add side modal heading #2533
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
Add side modal heading #2533
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
benjaminleonard
commented
Nov 6, 2024
<ProtocolField control={control} protocol="TCP" /> | ||
<ProtocolField control={control} protocol="UDP" /> | ||
<ProtocolField control={control} protocol="ICMP" /> | ||
</fieldset> | ||
|
||
<div className="flex flex-col gap-3"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stuff is unnecessary because the overall thing already has the spacing, it also causes a bit of havok since the spacing here is different.
david-crespo
reviewed
Nov 6, 2024
david-crespo
added a commit
that referenced
this pull request
Nov 6, 2024
* Reorder IDP create form elements * Update edit form order * Add docs link for Identity Providers * Remove checkbox for Signed Requests * Add side modal heading (#2533) * Add side modal heading * Remove commented input legend * very important mt-2 * cut one word to cut a whole line out of the targets info box --------- Co-authored-by: David Crespo <[email protected]> * change form section headings to SideModal.Heading, remove "General" --------- Co-authored-by: Benjamin Leonard <[email protected]> Co-authored-by: David Crespo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://console-git-side-modal-title-oxidecomputer.vercel.app/projects/mock-project/vpcs/mock-vpc/firewall-rules-new
Added
SideModal.Heading
– example usage on Firewall Rules side modal to be used also within #2511.The size looks good to me, making it semi seems to have made the difference in distinguishing it.
Also in terms of usage – I think a heading should always be preceded by a divider, things are going to get messy if we drop it in to group inputs willy nilly.