Skip to content

Conversation

@MarcelRobitaille
Copy link
Collaborator

@MarcelRobitaille MarcelRobitaille commented Jul 20, 2022

Fixes: #896

Fix this UI glitch by replacing the Breadcrumbs element with an element better suited for this kind of button layout. A ResponsiveActions was developed based on the nextcloud/vue Actions component. This ResponsiveActions shows the buttons with an icon and text if there is enough space, only the icons if there is only enough space for that, and only a 3-dot menu if very narrow devices like phones. This is to avoid mystery meat navigation.

Peek.2022-06-08.02-50.mp4

@github-actions
Copy link

github-actions bot commented Jul 20, 2022

Unit Test Results

     36 files       36 suites   10m 10s ⏱️
   402 tests    402 ✔️ 0 💤 0
4 824 runs  4 824 ✔️ 0 💤 0

Results for commit 45dc107.

♻️ This comment has been updated with latest results.

@MarcelRobitaille MarcelRobitaille force-pushed the 896-ui-glitch-with-narrow-devices branch from 9c4f4c2 to ba15a1d Compare July 20, 2022 22:20
@MarcelRobitaille
Copy link
Collaborator Author

I don't know where the PHP error is coming from. I didn't change any PHP.

@MarcelRobitaille
Copy link
Collaborator Author

MarcelRobitaille commented Jul 20, 2022

Also, not sure how copying files from nextcloud/nextcloud-vue works. It's GPLV3, so we probably need to state the changes and disclose the source. I'm not sure how that works though, I've never done that before.

src/assets/variables.scss is copied unchaged. ResponsiveActions is based on Actions.

@christianlupus
Copy link
Collaborator

I just compiled it and found a small further glitch:
grafik

To reproduce:

  1. Open recipe for editing. Refresh page to reload any JS code.
  2. Scale window such that "folding" begins but text (barely) present
  3. Save recipe causing more buttons to appear
  4. Rescaling seems to use the with of the original buttons (2 buttons during save) as a boundary for further folding.

@christianlupus
Copy link
Collaborator

Also, not sure how copying files from nextcloud/nextcloud-vue works. It's GPLV3, so we probably need to state the changes and disclose the source. I'm not sure how that works though, I've never done that before.

This is a tougher one, I fear. If they published the file under GPLv3, it is in general compatible with our AGPL. The problem is that once you copy and modify their code, you would have to publish under a compatible license. For GPLv3 that might be v3 or any successor. Enforcing another license (AGPL) might cause trouble. I am no lawyer, so I might be completely wrong. I am just saying, I would be very careful in this case.

One safe way would be to create our own fork of the repo, make the changes there, use the forked repo in the cookbook app and report the changes back. Then the team in the main Vue repo can consider whether the changes are worth getting integrated or not. GPL satisfied.
The drawback is obviously a massive amount of manual work on our side (keeping versions in sync).

I do not know what modifications you made exactly. Could this be incorporated into the main Vue library? Was e.g. a PR a feasible way? I will try to get in contact with the maintainers of the nextcloud-vue repo. Let's see what they are thinking about this.

@MarcelRobitaille
Copy link
Collaborator Author

Thanks for the bug report. I will try to reproduce and fix it.

I understand. I know licensing is tricky. I think it could be made into a PR. My issue requesting this upstream was completely ignored, but maybe a PR would be different.

Previously, we introduced a class `.breadcrumbs` that sets `width:
calc(100% - 60px)` to compensate for the left margin. However,
`.breadcrumb` defined in `@nextcloud/vue` Breadcrumbs
sets `width: 100%`. This was getting priority over our `calc` rule.
Not wanting to use `!important`, the solution I chose was to move this
single rule to the unscoped style tag and to add additional class name
selectors in order to increase the specificity.

Signed-off-by: Marcel Robitaille <[email protected]>
Implement a custom ResponsiveActions based on nextcloud/vue Actions
that automatically changes the button layout based on available space.

Signed-off-by: Marcel Robitaille <[email protected]>
Signed-off-by: Marcel Robitaille <[email protected]>
Signed-off-by: Marcel Robitaille <[email protected]>
This was only needed for customizing Breadcrumbs, which has been
removed.

Signed-off-by: Marcel Robitaille <[email protected]>
@MarcelRobitaille MarcelRobitaille force-pushed the 896-ui-glitch-with-narrow-devices branch from fe92aa2 to 45dc107 Compare July 23, 2022 05:54
@MarcelRobitaille
Copy link
Collaborator Author

Closing this because I think we're going with #1105

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.

UI glitch with narrow devices

2 participants