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

127-file multiselection action #217

Closed
wants to merge 3 commits into from

Conversation

TSI-kavitasonawane
Copy link

This PR contains the following changes:
File action button behaviour changes:

We now show more single actions depending on the viewport size in the table header beginning from the first option in the dropdown.
We add a new option called "Cancel" (ger: Abbrechen") which will remove any selection (just remove selection, not the files of course).
We right align the Actions button.
We will show every option in the action button dropdown. It doesnt matter which option is also shown in the table header.
We add a new label called "All".
on S: we hide the label "All" according to the screen design

@TSI-kavitasonawane TSI-kavitasonawane added the custom MagentaCLOUD customisation label Apr 28, 2023
@TSI-kavitasonawane
Copy link
Author

We had already raised as PR for upstream to nextcloud but they have closed it ..
so still we need to try to upstream these feature changes ??
PR link: nextcloud#28834

@tsdicloud tsdicloud marked this pull request as draft April 28, 2023 17:01
@tsdicloud tsdicloud added the build-ready Customization to include into build label May 5, 2023
@tsdicloud tsdicloud marked this pull request as ready for review May 5, 2023 14:21
@tsdicloud tsdicloud added build-ready Customization to include into build and removed build-ready Customization to include into build labels May 5, 2023
@TSI-kavitasonawane TSI-kavitasonawane force-pushed the nmc/127-file-multiselection-action branch from 808f333 to caaf75d Compare May 8, 2023 13:15
@tsdicloud
Copy link
Member

Still in acceptable many changed lines on core, though most of it is extension.
Waiting for a design that minimizes server core changes. Extensions can always be shifted to theme or nmc_sharing app.

@tsdicloud
Copy link
Member

Would also recommend to re-read the new delivery model concept. An accepted label does not say anything about your change is only there to include code in server build. (renamed it to build-ready to make this absolutely clear.

ATM, I expect a complete restart for this change as nothing has improved compared to V24 according to the new "Minimized change" rules.

@tsdicloud
Copy link
Member

Parts of the merge look specific to old grid layout.

@tsdicloud tsdicloud added redesign-for-upstream and removed build-ready Customization to include into build redesign-for-upstream labels May 14, 2023
@TSI-kavitasonawane
Copy link
Author

We raised PR for upstream to nextcloud .
PR link: nextcloud#38123

@tsi-nmc-build
Copy link

Kavita, STOP raising pull requests wildly.
This unclean code will never ever be accepted upstream in this way.

The obvious, quality way of raising an PR is:

  • Split code into the aspect only that has to be corrected upstream
    It is not acceptable that:
    (a) other aspects are mixed in
    (b) fragments of own customization are contained additionally
    (c) the whole thing has to work perfectly on our side with the current master of Nextcloud
    (d) quality has been proven by Javascript unit tests
    (e) Describe in detail the improvement in the CR and why it is mandatory for Nextcloud upstream.
    (f) There will be no takeover of Nextcloud staff for thePR to finalize.
    So it will take approximately the same effort again to adopt the upstream wishes to PR.
    I don't think this is what will help you at the moment.

    So please do your job and develop clean code that is bullet proof first.

@tsdicloud
Copy link
Member

I don't see that the finished PR is handled appropriately as the cancel code is still in the master patch.

As soon as Nextcloud contains an upstream patch, the refactoring steps are:
(1) Take out the aspect from the master patch or close the master patch PR if not needed anymore.
(2) Take the new code from Nextcloud and backport it for the older versions if needed.
We will review the backport approach this week, but in general, a backport is a branch with the same name as the master branch but with a -<version> suffix. So, if your master patch name is nmc/4711-my-pr-raise-branch, then
the backport name for V25 is like nmc/4711-my-pr-raise-branch-25 (from the new build process, will be a change in the concept this week)
(3) Test backport on older release.

I don't see any care for the nextcloud#28834 yet.

@tsdicloud tsdicloud added the build-ready Customization to include into build label May 16, 2023
@TSI-kavitasonawane TSI-kavitasonawane force-pushed the nmc/127-file-multiselection-action branch from caaf75d to 7b32738 Compare May 18, 2023 07:59
@tsdicloud tsdicloud added build-ready Customization to include into build and removed build-ready Customization to include into build labels Jun 13, 2023
@tsdicloud tsdicloud force-pushed the master branch 2 times, most recently from d0748cd to 8c7e2cd Compare June 14, 2023 17:46
tsi-nmc-build pushed a commit that referenced this pull request Jun 19, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 19, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 20, 2023
tsi-nmc-build pushed a commit that referenced this pull request Jun 21, 2023
@tsdicloud tsdicloud force-pushed the master branch 4 times, most recently from ac0c03f to ec3289d Compare June 25, 2023 06:40
@tsdicloud tsdicloud removed the build-ready Customization to include into build label Jun 25, 2023
@tsdicloud
Copy link
Member

Overwrite of Nextcloud code, should be an extension as add-on functionality to Nextcloud standard.

@tsdicloud tsdicloud added the invalid This doesn't seem right label Jun 29, 2023
@tsdicloud tsdicloud added iteration25 Experimental release 25 (review material only) and removed custom MagentaCLOUD customisation labels Jul 18, 2023
@tsdicloud
Copy link
Member

Needs redo, review only.

@tsdicloud tsdicloud closed this Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right iteration25 Experimental release 25 (review material only)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants