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 styles for application keys form #3942

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix styles for application keys form #3942

wants to merge 2 commits into from

Conversation

mayorova
Copy link
Contributor

@mayorova mayorova commented Nov 13, 2024

What this PR does / why we need it:

Not sure this is the right place to put this... but seems to be working as expected.

Before:

Screenshot from 2024-11-13 15-19-57

After:

Screenshot from 2024-11-13 15-17-25

It is especially noticeable when there is an error.

Also:

Before:
Screenshot from 2024-11-14 10-35-15

After:

Screenshot from 2024-11-14 12-28-50

Which issue(s) this PR fixes

Verification steps

Special notes for your reviewer:

@mayorova mayorova changed the title WIP: Fix styles Fix styles for application keys form Nov 14, 2024
@mayorova mayorova marked this pull request as ready for review November 14, 2024 11:39
jlledom
jlledom previously approved these changes Nov 26, 2024
Copy link
Contributor

@jlledom jlledom left a comment

Choose a reason for hiding this comment

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

It looks much better now.

About the CSS, I have so many questions:

  • Is _tables.scss the proper place to add the new style?
  • Is the new style needed? Maybe the new style is not needed because Patternfly itself already includes a class for that.
  • Is removing the border actually needed? Apparently tables look like that in PF.
  • Is a table the proper element to arrange buttons?

I don't know the answers so I'll approve since it looks good. But maybe we should ask UX people about this.

@mayorova
Copy link
Contributor Author

@jlledom why are you asking so many uncomfortable questions?... It was not me who same up with the idea of using a table row for organizing the buttons... but you can check the latest version, I hope you like it more.

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.

2 participants