Remove Banner for INSUFFICIENT_STORAGE while in device plugin #11809
Conversation
Build Artifacts
|
iskipu
changed the title
github-actions
bot
added
APP: Learn
|
The code read-through looks good, thanks @iskipu for revising this and for your patience. @radinamatic could we do a quick manual QA here? I am actually thinking this might be good to merge into 0.16.0 before release, and would appreciate the extra eyes on it in case we do. I'll chat to @rtibbles about it today if he's back. |
There was a problem hiding this comment.
This is implemented as specified in #11774, specifically
the banner now appears in Learn when the status is INSUFFICIENT_STORAGE, gets dismissed after a user clicks "Manage channels" and the banner doesn't appear in the Device plugin or anywhere else besides the Learn plugin.
I have reported separately the following issue #11866 which is not caused by the changes made here.
|
Hi @pcenov i have made changes to fix the issues mentioned by you (i.e adding space between the buttons and making the my download button work). fix-2024-02-20.mp4However i have one doubt:
What is the exact design i should implment bcz the changes i made just adds space between the buttons but doesnt address how it looks in a mobile device
|
There was a problem hiding this comment.
Thanks @iskipu - I confirm that now there's space between the buttons and clicking on 'Go to my downloads' brings the user to the correct page.
@marcellamaki could you please advise on the mobile design issue?
There was a problem hiding this comment.
Hi @iskipu - thanks for your work on this and for the QA @pcenov.
For the mobile issue, I think the best approach would be for the buttons to stack vertically, for example
[CLOSE]
[GO TO MY DOWNLOADS]
[MANAGE CHANNELS]
Depending on the language that Kolibri is in, these buttons can be quite long! So, on mobile, giving them their own space rather than trying to predict how wide they would be is probably a good idea.
This will make the banner a bit taller, and thus it will cover more of the screen, but I think that is okay for now! We can revisit the design in more detail later if it becomes problematic
|
Hi @pcenov made changes as per the suggestion given by @marcellamaki Mobile view:
Tab/Laptop/Computer view:
|
There was a problem hiding this comment.
I confirm that now the buttons are positioned vertically but the responsive design experience is not perfect and also there's not enough space between the buttons. Still this could be good enough for the time being and we can have the issue addressed separately @marcellamaki - could you have a look as well?
2024-03-05_12-35-55.mp4
|
Hi @pcenov thanks for the feedback. In the earlier commit i was too focused on mobile and desktop layout and haven't checked the whole spectrum of widths XD. Now i have fixed the responsiveness and also increased the spacing between the buttons. responsive.mp4 |
Fixes the issue #11774
Solves all the issues mentioned in the issue's checklist
Hope this helps