Skip to content

UI glitch with narrow devices #896

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

Closed
christianlupus opened this issue Feb 20, 2022 · 36 comments · Fixed by #1105
Closed

UI glitch with narrow devices #896

christianlupus opened this issue Feb 20, 2022 · 36 comments · Fixed by #1105
Assignees
Labels
bug Something isn't working Frontend Issue or PR related to the frontend code javascript Pull requests that update Javascript code

Comments

@christianlupus
Copy link
Collaborator

Description
At least when viewing a recipe the navigation is not handling the options correctly.

Screenshots
If applicable, add screenshots to help explain your problem.
screenshot at desktop
screenshot at narrow device

The reload, edit and print actions are obviously not visible on the narrow device.

Browser
Which browser are you using? Firefox

@christianlupus christianlupus added bug Something isn't working javascript Pull requests that update Javascript code Frontend Issue or PR related to the frontend code labels Feb 20, 2022
@MarcelRobitaille
Copy link
Collaborator

I would like to try this one. Should the icon buttons stay in the panel? It looks like there is space for them once the text goes into an expanding menu.

@seyfeb
Copy link
Collaborator

seyfeb commented Mar 5, 2022

I'd say it wouldn't hurt to have them directly accessible and not hidden behind an additional burger menu if there is sufficient space

@MarcelRobitaille
Copy link
Collaborator

How about the folder icons with no text next to them in the screenshot? Is that part of this issue?

@seyfeb
Copy link
Collaborator

seyfeb commented Mar 6, 2022

I think this is related. If I remember correctly, the actions (print, delete, edit) were added as breadcrumbs. My guess is, that they are hidden within the dropdown as a folder icon with no text.

@MarcelRobitaille
Copy link
Collaborator

I am not able to reproduce this unless I make my viewport extremely small (243px). Before that, the recipe title is still visible, so there is no space for the buttons in the right.

@christianlupus, May I ask what your resolution is when experiencing this issue?

image

Here is a video with different sizes

Peek.2022-03-07.13-34.mp4

There also seems to be a related problem around 5 seconds where one button goes out of the viewport to the right. Should I open a new issue for this or do you think that this has the same root cause?

@seyfeb
Copy link
Collaborator

seyfeb commented Mar 7, 2022

I can confirm the issue on my Samsung galaxy S9 which has a resolution of 2960 × 1440 but the logical resolution is obviously different. screen.width returns 408px

@MarcelRobitaille
Copy link
Collaborator

I think Breadcrumbs may be the wrong component for this. From what I have seen, this is for displaying the ancestry of a folder. Here is the example from the docs:
image

The problem with using this is that it makes a log of assumptions. For example, when collapsing, it assumes that the best folders to hide are the middle ones, leaving the root folder and deepest folder visible.Maybe I am wrong, but I don't see any way to control this.

What does everyone think of re-implementing this ourselves? The styling would not be difficult, and we would have fine control on which buttons are hidden at which screen size. This would also give us the opportunity to remove the arrows between the action buttons, which in my opinion should not be there.
image
Maybe we could instead use a simple line similar to @nextcloud/vue <ActionSeparator>, or just some space.

@seyfeb
Copy link
Collaborator

seyfeb commented Mar 8, 2022

The actions should definitely not be part of the breadcrumbs. I guess it was done as a quick 'n dirty workaround for getting some buttons up there.

However! I would keep the breadcrumbs for indicating the structure, e.g. for categories

Screen Shot 2022-03-08 at 20 59 04

or for things like search

Screen Shot 2022-03-08 at 20 58 56

Who knows, there were at some point discussions about maintaining multiple cookbooks in the app, then we might want to have a Nextcloud-styled breadcrumbs bar Grandmas recipes > Pasta > Spaghetti Bolognese when viewing the recipe.

The actions can be separated from the breadcrumbs container and added, e.g., floating to the right and might even be hidden in an Actions component for small screen widths

Screen Shot 2022-03-08 at 20 55 21

(Idk, why the reload icon for the recipe is overlapping with the text. Is it like this this just for me?)

@MarcelRobitaille
Copy link
Collaborator

@seyfeb Thanks for your thorough response.

(Idk, why the reload icon for the recipe is overlapping with the text. Is it like this this just for me?)

This also changed for me as well recently in my development instance. I'm not sure what would have changed it. Maybe a regression in @nextcloud/vue?

@MarcelRobitaille
Copy link
Collaborator

This is what I have now:
image

I put the action buttons on the right for now, but they could easily be moved to be left-aligned with everything else.

The problem now is that the recipe name / reload button is never collapsing into the menu because the Breadcrumbs component never collapses the first or last item. I think it would be better to collapse all of the action buttons into a menu so that the title is always visible. I don't see a way to do this with the default components in @nextcloud/vue, so I opened an issue.

This becomes a problem around 400px.

Peek.2022-03-09.11-53.mp4

Sorry about the video. It's a little bit hard to tell the problem with the icon position bug that @seyfeb mentioned. Please notice the rounded corners working down to 400px, but then becoming square as the action buttons to the right take that space.

@christianlupus
Copy link
Collaborator Author

I have the same issue with the reload button. Do you wish to solve the issue as a by-product or open a separate issue to keep track of it, @MarcelRobitaille?

@MarcelRobitaille
Copy link
Collaborator

I don't think that is the same issue. I think the reload button might be a bug in @nextcloud/vue. It showed up in master sometime last week. I can try to track down exactly which commit to confirm.

@MarcelRobitaille
Copy link
Collaborator

I installed @nextcloud/vue 4.0.2 and confirm that the reload button is fixed. This bug was introduced in ab74aa8 and I think it's a bug with @nextcloud/vue.

@christianlupus
Copy link
Collaborator Author

No, I meant that if you put the buttons in their own container, the reload button might be the best fitting there as well. That might solve the glitch with the overlapping icon as a byproduct.

@MarcelRobitaille
Copy link
Collaborator

Oh I see, sorry. I think that could be this same issue then.

@MarcelRobitaille
Copy link
Collaborator

I moved the reload button with the other action buttons. This is how it looks now.

image

Still waiting on nextcloud-libraries/nextcloud-vue#2541 to make it responsive.

@seyfeb
Copy link
Collaborator

seyfeb commented Mar 19, 2022

I think this should be fine. I never felt the need to use the reload button anyways. is there an actual need/ use case for this? 🤔

@MarcelRobitaille
Copy link
Collaborator

I am not sure. Maybe if you edit the recipe in a different tab? I agree that this button need not be so readily available. Maybe we could show a three-dot menu to the right of all of the buttons with only the reload button, and progressively move buttons from the bar into the expanding menu as the window width decreases. I think we could already do this without changes to @nextcloud/vue. We might have to duplicate some of the buttons in the DOM, though. What do you think?

@seyfeb
Copy link
Collaborator

seyfeb commented Mar 19, 2022

A hamburger menu with a single action doesn’t sound like a good idea. I think showing all buttons and going with your suggested approach with decreasing screen width:

full button (with labels) > only icon buttons > 3-dot menu

is (going to be) fine. I was only thinking loudly when asking if there is a use case for the reload button in case someone has a need

@christianlupus
Copy link
Collaborator Author

At least for debugging it is useful. Having a button to abort the edits might be the alternative. This is something I am missing when navigating intuitively.

But yes, I was not rejecting the idea to reduce the with in steps as @seyfeb summarized. I am more thinking to add all "action buttons" (save, reload, delete, edit, ...) in one location.

@MarcelRobitaille
Copy link
Collaborator

It doesn't seem like that nextcloud-vue issue is going to get addressed anytime soon, so I decided to create my own <ResponsiveActions> component.

Here is what I have so far:

Peek.2022-06-08.02-50.mp4

One issue I am facing is that there is an issue with the tooltips for the intermediate mode. I am not sure if anybody here knows how to fix that.

I also have to fix the error that sometimes comes up when the click event is undefined. nextcloud-vue handle this by creating a new method that calls the only visible button if its click action exists. This is more difficult in our case because we have more than one action.

Finally, there was a glitch in the video where the buttons got small even though there was plenty of space. I never noticed this until recording the video. I think it is just a css glitch.

The code is still a work in progress. I have to clean some things up, figure out how to deal with importing things that aren't exported from nextcloud-vue, and reset my commits and create new ones with real messages. Nonetheless, my WIP code is here: https://github.com./MarcelRobitaille/cookbook/blob/896-ui-glitch-with-narrow-devices/src/components/ResponsiveActions.vue. I also had to add key props to the <Actions> here: https://github.com./MarcelRobitaille/cookbook/blob/896-ui-glitch-with-narrow-devices/src/components/AppControls.vue#L120-L185

@christianlupus
Copy link
Collaborator Author

@MarcelRobitaille how much effort do you guess it will take until this might be running?

I am thinking of the next release and if we should include this in the release or not.

@MarcelRobitaille
Copy link
Collaborator

I was able to fix the tooltip issue. I just had to specify boundariesElement: 'body' in the v-tooltip options. I also cleaned up the code a little bit by adding an enum. Since I we're discussing possibly releasing soon, I created a pull request and redid my "WIP" commits with the proper sign off. What do you think?

@christianlupus
Copy link
Collaborator Author

Do not worry about the timeline. Better having good code slowly but bad code that needs complete rewriting.

I will have to take a closer look here. Will come back to you soon.

@nimishavijay
Copy link
Member

It doesn't seem like that nextcloud-vue issue is going to get addressed anytime soon, so I decided to create my own component.

@MarcelRobitaille really cool work! The reason there's no component for something like this is because we generally don't have a row of many buttons with text, it's usually one or two commonly used buttons :)
Over here, since I assume the "Edit" button is the one which would be used the most, that could be outside, and the rest could go in a 3dot menu.
Collectives is an example of an app that does this
image

You could also just use a row of icons without the text, and if the screen is still too small, use a 3 dot menu. Deck does something similar.
image

I would suggest since the reload and delete actions are not used very often they can go into a 3 dot menu either way :)

@MarcelRobitaille
Copy link
Collaborator

@nimishavijay Thanks for the comment. That's another way to do it. I must admit, it looks good in your screenshots. I like the blue primary button. I am open to this, but I wonder what the others think. I agree that for me, the most used button would be edit.

@TheMBeat
Copy link
Contributor

I think the Edith button and the ...button look a little better. It doesn't look so cluttered. Still, I think your implementation is cool too.

@MarcelRobitaille
Copy link
Collaborator

I made a quick implementation of @nimishavijay's proposal (thanks for the inspiration 😁).

Peek.2022-07-23.03-07.mp4

I think it looks really good. I like the idea of having one primary button and hiding the less common buttons to keep the UI clean and uncluttered.

However, it looks like we may still need some responsiveness in this implementation. Especially when "Edit recipe" is visible, the "Save changes" button can overflow. We should consider changing this to icon-only and/or moving it to the overflow menu dynamically, but then we're back to <ResponsiveActions>. This may be solved by removing the breadcrumb component as discussed in other issues. We can also truncate the recipe title, which we will also need to do for long titles. Right now, it is kind of working, but the recipe title "wins" the flex-shrink war.

https://github.com./MarcelRobitaille/cookbook/pull/new/896-primary-edit-button

@christianlupus
Copy link
Collaborator Author

My comments on this:

Maybe we can replace the texts Save changes and Edit recipe by simply Save and Edit. That might make more space.

For the responsiveness question: How about a CSS based solution?

  • For wide displays (more than approx 350px) we have the solution as in your video from today. You could pack the Action components into an absolutely positioned block that will overflow in the worst case the title (the title is visible just below so no big problem). This behavior is similar to many other apps, I tested recently.
  • For narrower devices one could consider adding a second row with just the primary (read edit) button and the three dot menu. This is mainly to keep the navigation available for the most narrow devices.

Would you think that manageable?

@MarcelRobitaille
Copy link
Collaborator

Maybe we can replace the texts Save changes and Edit recipe by simply Save and Edit. That might make more space.

I thought about it, but then we would have a lot of translations to update, right?

I will see what I can do for the responsiveness. That's a good idea

@christianlupus
Copy link
Collaborator Author

I thought about it, but then we would have a lot of translations to update, right?

Yes, there are many translations to be fixed. But this will come as time passes. It is not our primary motivation to keep the texts as stable as possible. Technical functionality has more importance.

But the translations are made by a community themselves. Thanks for that. This is quite some work, but as the group is quite large that will not take very long. A few days or some weeks and everything should be translated.

@MarcelRobitaille
Copy link
Collaborator

MarcelRobitaille commented Jul 23, 2022

I went all the way and took out the breadcrumbs all together. Instead of having Home > Edit > Recipe name, now I have "Editing recipe" or "Viewing recipe" above "Recipe name". I think this is more consistent (shows the mode when viewing and editing) and it saves space.

I took out the home button, but I think this is made redundant by the "Cookbook" button in the Nextcloud top bar or the "All recipes" button in the sidebar.

I made the recipe title become truncated with ellipsis if there is not enough space.

The result is that the same layout can be kept even with very narrow screens.

This also fixes #281

Peek.2022-07-23.16-57.mp4

There are still some translations to update

Forgot to show it in the video but it works for category too:

image

@christianlupus
Copy link
Collaborator Author

That video looks really good! Wow! 🎉😎

I want to try that out tomorrow if you do not mind. But I think this is the cleanest solution i have seen in the app so far.

@MarcelRobitaille
Copy link
Collaborator

Thanks! I pushed everything and created a draft PR so you can easily find my branch when you check it out.

@seyfeb
Copy link
Collaborator

seyfeb commented Jul 24, 2022

Well done! 👏 Just one thing to add: I think "Viewing Recipe" is a little redundant, since you most probably know that you are viewing a recipe and, if not, you can figure it out by the fact that there is the edit button. I think removing this might reduce clutter and look even cleaner. Just my opinion though and maybe sth to discuss separately.

A second thing: I feel like the new element could use a little bit of white space at the bottom so its easier to visually separate it from the title.

Those are very minor comments though 😅

@MarcelRobitaille
Copy link
Collaborator

Thanks @seyfeb.

you can figure it out by the fact that there is the edit button.

By the same token, you should figure out that you are editing by the save button. I think it's better to always show the mode when applicable for consistency. I am willing to remove "Viewing recipe" though.

A second thing: I feel like the new element could use a little bit of white space at the bottom so its easier to visually separate it from the title.

I added the maximum space by setting line-height = font-size and justify-content: space-between;. In this extreme case, I think it looks bad.

image

Here is space-around

image

How much space did you want between the two?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Frontend Issue or PR related to the frontend code javascript Pull requests that update Javascript code
Projects
None yet
5 participants