-
-
Notifications
You must be signed in to change notification settings - Fork 898
feat: Allow viewing patches without needing app installed #2699
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
base: dev
Are you sure you want to change the base?
Conversation
Got a screenshot of how it looks like/where this function is added |
the view model needs to be parametrized to not include the checkmark or allow editing options. basically a readonly param |
pov main branch |
Usually i would agree but this repo is so close to depreciation that it shouldn't be considered. I tried to achieve it with as low code as possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥞👨💻 Close enough, welcome back cossale to the team.
Co-authored-by: Pun Butrach <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this repo is so close to depreciation that it shouldn't be considered. I tried to achieve it with as low code as possible.
Regardless, the change should be done, its merely a couple of if statements based on an injected boolean to hide UIs
@@ -73,6 +73,14 @@ class _NotInstalledAppItem extends State<NotInstalledAppItem> { | |||
), | |||
], | |||
), | |||
Text( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI language doesnt match with the rest of the surrounding UI. Please resolve this with e.g. @validcube
i feel like that would be too long. and cancel/ok doesn't make sense because there is nothing to cancel there |
Since tap to view patches is now added, the list of patches should be moved somewhere near to it. Right now its in the title next to the app name, it should be moved to a line for example: View patches (1) |
cc: @PalmDevs UI/UX |
I think having dialog blocking an action that isn't destructive isn't ideal. I'd simply show a highlighted card at the top of the page with similar contents. |
No description provided.