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: update modal buttons alignments in element dialogs #3365

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

Conversation

tjuanitas
Copy link
Contributor

@tjuanitas tjuanitas commented Jun 21, 2023

This is updating the designs of the modals used in Content Explorer and Content Picker. There are three modal components that this is specifically targeting: CreateFolderDialog.js, DeleteConfirmationDialog.js, and RenameDialog.js

I decided to add width: 100%; to all text fields in the base elements stylesheet since 100% seems to be the most ideal value for width in majority of cases for input.

At the moment, components are inheriting width: 262px from the base components stylesheet but this seems to be an arbitrary value and components are overriding it. e.g.

Before
Screenshot 2023-07-05 at 11 19 50 AM

After
Screenshot 2023-07-05 at 11 19 35 AM

@tjuanitas
Copy link
Contributor Author

note to self: the textarea in USM seems to render immediately while the rest of the UI is loading

@@ -1,6 +1,6 @@
@import '../variables';

.be .be-header {
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 .be on this selector breaks the .be-is-small & selector below

Comment on lines -20 to -24
input[type='search'] {
width: 100%;
font: inherit;
-webkit-appearance: none;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not too sure what these lines were intended for. my assumption is this was related to a Safari issue but we don't target input[type='search'] for any other components so I don't know why this was needed.

I think it's okay to remove the block now:

@@ -23,7 +22,8 @@
input[type='number'],
div[contentEditable='true'],
textarea {
border-color: $bdl-gray-50;
width: 100%;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding width: 100% here can be a little risky since it'll apply the style to all text fields in all the elements.

I did testing in all the elements but haven't seen an issue yet. most uses of inputs/textareas are already using width: 100%. e.g:

@tjuanitas
Copy link
Contributor Author

I'll continue more testing of the elements to verify width: 100% has no unintended side effects but PR is ready for review now.

@tjuanitas tjuanitas marked this pull request as ready for review July 5, 2023 18:37
@tjuanitas tjuanitas requested review from a team as code owners July 5, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant