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: [M3-6606] - Add spacing between copy and Linode Rebuild button. #10283

Merged
merged 7 commits into from
Mar 25, 2024

Conversation

cpathipa
Copy link
Contributor

Description πŸ“

Add's spacing between copy and Linode Rebuild button.

Before After
image image

How to test πŸ§ͺ

Verification steps

(How to verify changes)

  • Navigate to Linode Rebuild form scroll down and observe the spacing for Rebuild Linode button.
  • ...

As an Author I have considered πŸ€”

Check all that apply

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@cpathipa cpathipa requested a review from a team as a code owner March 14, 2024 15:51
@cpathipa cpathipa requested review from dwiley-akamai and hana-linode and removed request for a team March 14, 2024 15:51
@cpathipa cpathipa self-assigned this Mar 14, 2024
@cpathipa cpathipa marked this pull request as draft March 14, 2024 15:51
@cpathipa cpathipa added the Bug Fixes for regressions or bugs label Mar 14, 2024
@cpathipa cpathipa marked this pull request as ready for review March 14, 2024 15:53
@bnussman-akamai
Copy link
Contributor

Can we make the button right aligned? (flex-end)

@bnussman-akamai
Copy link
Contributor

Also, because there is only one button, I think it would be a lot cleaner to just use a plain "Button" (no ActionsPanel) that is wrapped in a flexbox.

@jdamore-linode
Copy link
Contributor

Can we make the button right aligned? (flex-end)

If we did that, I wonder if it would look weird with the left-aligned "Add an SSH Key" button right above? (Or would you suggest right-aligning that too?)

Also, because there is only one button, I think it would be a lot cleaner to just use a plain "Button" (no ActionsPanel) that is wrapped in a flexbox.

+1

Copy link

github-actions bot commented Mar 14, 2024

Coverage Report: βœ…
Base Coverage: 81.65%
Current Coverage: 81.65%

@cpathipa
Copy link
Contributor Author

Can we make the button right aligned? (flex-end)

If we did that, I wonder if it would look weird with the left-aligned "Add an SSH Key" button right above? (Or would you suggest right-aligning that too?)

Also, because there is only one button, I think it would be a lot cleaner to just use a plain "Button" (no ActionsPanel) that is wrapped in a flexbox.

+1

I think we have a similar pattern of having some buttons on the left and some on the right, as seen in the Linode create flow. IMO, it would be beneficial to render action buttons with the ActionsPanel component since it supports rendering both primary and secondary action buttons. Also, it would enhance the maintainability of primary and secondary action buttons across the CM.

image

@cpathipa cpathipa added the Add'tl Approval Needed Waiting on another approval! label Mar 14, 2024
cpathipa and others added 2 commits March 14, 2024 18:39
Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
@hana-linode hana-linode added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Mar 18, 2024
@cpathipa cpathipa merged commit e4fe05b into linode:develop Mar 25, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants