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

Tracking banner #1063

Merged
merged 12 commits into from
Jan 6, 2024
Merged

Tracking banner #1063

merged 12 commits into from
Jan 6, 2024

Conversation

DGoiana
Copy link
Collaborator

@DGoiana DGoiana commented Dec 18, 2023

Closes #1047 and #1070
Tracking banner in order to inform people of the recent data tracking changes

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

@DGoiana
Copy link
Collaborator Author

DGoiana commented Dec 18, 2023

Screenshot 2023-12-18 at 16 46 35

Copy link

codecov bot commented Dec 18, 2023

Codecov Report

Merging #1063 (d4b34dd) into develop (8c7d071) will decrease coverage by 0%.
Report is 2 commits behind head on develop.
The diff coverage is 6%.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #1063   +/-   ##
=======================================
- Coverage       17%     17%   -0%     
=======================================
  Files          207     208    +1     
  Lines         6396    6432   +36     
=======================================
- Hits          1043    1037    -6     
- Misses        5353    5395   +42     

Copy link
Member

@bdmendes bdmendes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, @thePeras any input on the design?

uni/lib/view/home/widgets/tracking_banner.dart Outdated Show resolved Hide resolved
uni/lib/view/home/home.dart Outdated Show resolved Hide resolved
uni/lib/view/home/home.dart Outdated Show resolved Hide resolved
@thePeras
Copy link
Member

thePeras commented Dec 18, 2023

Also, @thePeras any input on the design?

This is a one time view?
If so, why not a dialog? With two options: "ok", "change". After that the UI, is cleaner

@thePeras
Copy link
Member

Also, @thePeras any input on the design?

This is a one time view? If so, why not a dialog? With two options: "ok", "change". After that the UI, is cleaner

Or better, why not an actually Banner?

@DGoiana
Copy link
Collaborator Author

DGoiana commented Dec 22, 2023

I've created two options, using native flutter MaterialBanner. One redirects the user directly to the settings page, although it occupies more space. The left one is simpler and cleaner but it does not have the option to redirect the user.

Screenshot 2023-12-22 at 15 30 20 Screenshot 2023-12-22 at 15 29 51

Copy link
Member

@thePeras thePeras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some observations for clean code

uni/lib/view/home/widgets/tracking_banner.dart Outdated Show resolved Hide resolved
uni/lib/view/home/home.dart Outdated Show resolved Hide resolved
uni/lib/view/home/home.dart Outdated Show resolved Hide resolved
@bdmendes bdmendes requested a review from thePeras January 5, 2024 15:38
@thePeras
Copy link
Member

thePeras commented Jan 6, 2024

✅ 🛫

@thePeras thePeras merged commit 26fb87c into develop Jan 6, 2024
6 checks passed
@thePeras thePeras deleted the feature/tracking-banner branch January 6, 2024 18:37
@DGoiana DGoiana mentioned this pull request Jan 12, 2024
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.

Tracking banner
3 participants