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

May be delete ui-corner-.. ? #2739

Closed
johnfort opened this issue Oct 21, 2018 · 11 comments
Closed

May be delete ui-corner-.. ? #2739

johnfort opened this issue Oct 21, 2018 · 11 comments
Labels

Comments

@johnfort
Copy link

johnfort commented Oct 21, 2018

In main container, toolbar, dialogs ?

@nao-pon
Copy link
Member

nao-pon commented Oct 22, 2018

@johnfort The basic design of elFinder adopts it because it is rounded round. Please adjust it with CSS to change it.

@johnfort
Copy link
Author

@nao-pon All rounds can be created through specific elfinder classes.
I use hard reset styles.
I propose to reduce the size of the code.

@nao-pon
Copy link
Member

nao-pon commented Oct 23, 2018

Even assigning own class name will specify the same style as CSS in jQuery-UI, so you either need to overwrite the CSS in either case. I think that there is not much difference.

ex.

.elfinder .ui-corner-all { border-radius: 0; }

or

.elfinder-ui-corner-all { border-radius: 0; }

@johnfort
Copy link
Author

johnfort commented Oct 23, 2018

@nao-pon You did not understand.
These classes already exist. See for example:

class="ui-helper-clearfix ui-widget-header ui-corner-top elfinder-toolbar"

if i want use border-radius, i can use rule .elfinder-toolbar.

p.s. Waiting for you to move "borders*, backgrounds, color" to elfinder.theme.css 👻

@nao-pon
Copy link
Member

nao-pon commented Oct 23, 2018

I will arrive at not using the "ui-*" class if I stick to your logic. That is because the boundary of what to eliminate is ambiguous. For example, margin, padding, display, etc. may be disturbing depending on theme. For now I think that theme.css is to define color. Please understand that applying the unique theme by overwriting it with the theme by jQuery-UI as a standard.

On the other hand, if there is something that can not be dealt with by overwriting it should be rectified, please let me know if there is such a point.

Also, elFinder was not thought about changing the theme at the beginning of development, so if we are going to start elFinder 3, I would like to have a structure that will become a separate CSS from logic and design.

@johnfort
Copy link
Author

johnfort commented Oct 23, 2018

@nao-pon

For now I think that theme.css is to define color.

Colors in text and background as similar jquery-ui.structure/jquery-ui.theme.

You use raster graphics, which is already outdated.
Therefore it is worth moving all the rules containing "background / color" to ".theme". This is exactly what your logic is.

I would like to have a structure that will become a separate CSS from logic and design.

Need html layout of the main elements. For example, as in https://github.com/dimsemenov/PhotoSwipe

@johnfort
Copy link
Author

@nao-pon You also need to add a script to style the scroll bars.

@nao-pon
Copy link
Member

nao-pon commented Oct 28, 2018

@johnfort For toolbar I found that it is okay to delete "ui - corner - *". I will delete it.

nao-pon added a commit that referenced this issue Oct 28, 2018
@nao-pon
Copy link
Member

nao-pon commented Oct 28, 2018

Again, I decided to revive the designation of "ui-corner-all" CSS class of toolbar. Instead, I'll remove "border-radius: 4px" from toolbar.css. The reason is that the designation of "ui-corner-all" is often used elsewhere, so removing it from toolbar has no meaning. There is the possibility of removing "ui-corner-*" in the future, but it is not now. ( 13c7a0e )

@johnfort
Copy link
Author

Than see L31 in elFinder/css/dialog.css

ui-dialog-titlebar ui-corner-all
to
ui-dialog-titlebar ui-corner-top

@nao-pon
Copy link
Member

nao-pon commented Nov 2, 2018

I will consider using jQuery-UI's CSS class in this next major version.

@nao-pon nao-pon closed this as completed Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants