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

display file list in flexbox #20201

Merged
merged 1 commit into from
Apr 17, 2020
Merged

Conversation

rmanibus
Copy link

@rmanibus rmanibus commented Mar 27, 2020

fixes #20130

@GretaD
Copy link
Contributor

GretaD commented Mar 27, 2020

Thank you @rmanibus for your contribution :)
The problem with buttons and scrolling is fixed, but now the upload button stays always below the path.
Before the change
move_before

After the change
move

@rmanibus
Copy link
Author

oh yes I missed that.
I will try something (sorry I'm not a frontend developer haha)

@rmanibus
Copy link
Author

Ok so the breadcrumb is an inline flex, I don't see how we can put the add button after the last breadcrumb item without putting it in the same flex context.

This is working if the breadcrumb is less than a line long, but if it is two line long the add button will not be on the same line than the last breadcrumb item.

<div style="display: inline-flex; flex-direction:row;">
  <span class="dirtree breadcrumb" style="flex-shrink: 1;">
    <div data-dir="" class="crumb"><a></a></div>
    <div data-dir="/..." class="crumb"><a>long name destination</a></div>
  </span>
  <span class="actions creatable">...</span>
</div>

I think the button must be the last breadcrumb item for it to work. Do you see any other solution ?

@rmanibus
Copy link
Author

rmanibus commented Mar 28, 2020

The main breadcrumb is also broken if the path is too long in 18.02

main breadcrumb

@GretaD
Copy link
Contributor

GretaD commented Mar 30, 2020

Ok so the breadcrumb is an inline flex, I don't see how we can put the add button after the last breadcrumb item without putting it in the same flex context.

This is working if the breadcrumb is less than a line long, but if it is two line long the add button will not be on the same line than the last breadcrumb item.

<div style="display: inline-flex; flex-direction:row;">
  <span class="dirtree breadcrumb" style="flex-shrink: 1;">
    <div data-dir="" class="crumb"><a></a></div>
    <div data-dir="/..." class="crumb"><a>long name destination</a></div>
  </span>
  <span class="actions creatable">...</span>
</div>

I think the button must be the last breadcrumb item for it to work. Do you see any other solution ?

Hey, thank you for rechecking, no, i dont have any better solution tbh.

@rmanibus
Copy link
Author

so, the breadcrumb shows up correctly but the button is non functioning for now. what do you think of this ? I did not commit the compiled file for now because it is generating a lots of noise, I think you will have to rebuild the JS.

@rmanibus
Copy link
Author

rmanibus commented Mar 30, 2020

Fetching the button with detach() effectively preserve all event handlers attached with it (https://stackoverflow.com/questions/29461969/appending-element-and-removing-it-destroys-all-event-handlers-in-jquery). So the button is functioning again. I think we are good now.

@rmanibus
Copy link
Author

Do you want me to commit all the generated js ? I dont now why it generated files in the app folder, I only made change in the core.

@gary-kim
Copy link
Member

You can just comment /compile amend / and and @npmbuildbot will come in and compile it for you :).

@rmanibus
Copy link
Author

/compile amend /

@rmanibus
Copy link
Author

rmanibus commented Apr 9, 2020

hi, can someone review this ?

@gary-kim gary-kim added 3. to review Waiting for reviews bug labels Apr 9, 2020
@gary-kim gary-kim requested review from skjnldsv and GretaD April 9, 2020 13:27
@skjnldsv skjnldsv added the design Design, UI, UX, etc. label Apr 15, 2020
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

code looks good
Hopefully we create our own new component for it soon :)

@GretaD GretaD removed the 3. to review Waiting for reviews label Apr 16, 2020
@GretaD
Copy link
Contributor

GretaD commented Apr 16, 2020

please squash it and resolve the conflicts. Thank you 👍 1

@skjnldsv skjnldsv added the 2. developing Work in progress label Apr 16, 2020
@rmanibus
Copy link
Author

/compile amend /

@rmanibus
Copy link
Author

I think we are good !

Signed-off-by: npmbuildbot[bot] <npmbuildbot[bot]@users.noreply.github.com>
@gary-kim gary-kim added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 17, 2020
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 17, 2020
@skjnldsv skjnldsv added this to the Nextcloud 19 milestone Apr 17, 2020
@skjnldsv skjnldsv merged commit 2a51a32 into nextcloud:master Apr 17, 2020
@welcome
Copy link

welcome bot commented Apr 17, 2020

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@skjnldsv
Copy link
Member

Thanks a lot @rmanibus 🤗

@rullzer rullzer mentioned this pull request Apr 17, 2020
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug design Design, UI, UX, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot move or copy file when the target path take more than one line
4 participants