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.

Sign up for GitHub

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

Jump to bottom

Scrap MB references from Sigarra #665

Merged
merged 18 commits into from
Jul 10, 2023
Merged

Scrap MB references from Sigarra #665

merged 18 commits into from
Jul 10, 2023

Conversation

Process-ing
Copy link
Contributor

@Process-ing Process-ing commented Dec 28, 2022 •
edited
Loading

(Now fully) closes #460
The app can now scrap and save references from Sigarra, and the visuals are implemented accordingly (see comments below).

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes new summary in whatsnew/whatsnew-pt-PT
  • Properly adds 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

@Process-ing Process-ing changed the title Added functionality to scrap and save references from Sigarra. Scrap MB references from Sigarra Dec 28, 2022
brunogomes30
brunogomes30 suggested changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@brunogomes30 brunogomes30 left a comment

Choose a reason for hiding this comment

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

Looks very good. Some minor suggestions

brunogomes30
brunogomes30 previously approved these changes Feb 1, 2023
View reviewed changes
Copy link
Contributor

@brunogomes30 brunogomes30 left a comment

Choose a reason for hiding this comment

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

Looks good to me 🚀🚀

@Process-ing
Copy link
Contributor Author

Process-ing commented Feb 4, 2023 •
edited
Loading

I merged feature/scrap_references_visuals into this branch. See Pull Request #697.

@Process-ing Process-ing force-pushed the feature/scrap_references branch 2 times, most recently from ccfefdb to 9ee05af Compare February 15, 2023 16:10
@Process-ing Process-ing force-pushed the feature/scrap_references branch from 9ee05af to 462ff0d Compare March 25, 2023 18:25
@Process-ing
Copy link
Contributor Author

I finally migrated this branch to providers!!

@Process-ing
Copy link
Contributor Author

I changed the reference display, like we had discussed in the meetings.
Screenshot_1681229573

@thePeras
Copy link
Member

thePeras commented Apr 11, 2023 •
edited
Loading

The values are nice, however I think It doesn't make sense to center align the title and use that red color.

@Process-ing Process-ing force-pushed the feature/scrap_references branch from 6c195ca to 5f87469 Compare April 19, 2023 14:27
@Process-ing
Copy link
Contributor Author

The values are nice, however I think It doesn't make sense to center align the title and use that red color.

I changed the title. Not it looks like this:

Screenshot_1681914568

@Process-ing Process-ing requested a review from thePeras April 19, 2023 14:33
thePeras
thePeras requested changes Apr 26, 2023
View reviewed changes
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.

Give user feedback when copying the values to clipboard with a toast message.

bdmendes
bdmendes reviewed Apr 26, 2023
View reviewed changes
Copy link
Collaborator

@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.

Besides the nitpick, make sure to update the PR description since the UI is implemented and the issue can be closed completely.

@Process-ing
Copy link
Contributor Author

Give user feedback when copying the values to clipboard with a toast message.

Done!

Screenshot_1682527910

thePeras
thePeras requested changes Apr 27, 2023
View reviewed changes
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.

Besides this last suggestion, everything looks good and ready to go! 🛫

@Process-ing Process-ing force-pushed the feature/scrap_references branch from 18626cb to 691e933 Compare April 28, 2023 08:07
@Process-ing Process-ing requested a review from thePeras April 28, 2023 08:23
@Process-ing Process-ing force-pushed the feature/scrap_references branch 2 times, most recently from 934b4a6 to 2dad32e Compare April 28, 2023 13:21
thePeras
thePeras previously approved these changes Apr 28, 2023
View reviewed changes
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.

Great work!! ✅ 🛫

@Process-ing Process-ing requested a review from thePeras June 20, 2023 12:02
thePeras
thePeras previously approved these changes Jun 21, 2023
View reviewed changes
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.

✅ 🛫

bdmendes
bdmendes reviewed Jul 3, 2023
View reviewed changes
Copy link
Collaborator

@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.

Very clean code! Looking great.
Just a minor nitpick before merging this, can we make the empty placeholder text a bit lighter and smaller (possibly with a bit more padding?)

image

@Process-ing
Copy link
Contributor Author

Process-ing commented Jul 3, 2023 •
edited
Loading

Very clean code! Looking great. Just a minor nitpick before merging this, can we make the empty placeholder text a bit lighter and smaller (possibly with a bit more padding?)

image

Do you like this way?

Screenshot_1688407967

@Process-ing Process-ing requested review from bdmendes and thePeras July 5, 2023 13:43
Process-ing and others added 18 commits July 10, 2023 16:40
@Process-ing @bdmendes
Added functionality to scrap and save references from Sigarra.
9a5cb42
@Process-ing @bdmendes
Added reference cards, displayed in the profile page.
3e47aee
@Process-ing @bdmendes
Updated references.dart and action_creators.dart
0a14b13
@Process-ing @bdmendes
Added copy button to reference infos. Changed reference card text style.
a37a350
@Process-ing @bdmendes
Migrated reference functionality to Providers.
f0841aa
@Process-ing @bdmendes
Removed "Data limite" from reference card.
2753305
@Process-ing @bdmendes
The app now only shows the two most urgent references.
f734f91
@Process-ing @bdmendes
Changed reference display format.
ec74bdd
@Process-ing @bdmendes
Changed reference display format (again).
2cd2ce8
@Process-ing @bdmendes
Changed reference display title.
f535538
@Process-ing @bdmendes
Added toast message when copying reference info.
e94a86b
@Process-ing @bdmendes
Personalized toast message when copying reference info.
8eb7d53
@Process-ing @bdmendes
Moved reference widgets to its own class.
1d0c867
@Process-ing @bdmendes
Changed reference styles (updated deprecated).
6d5753b
@Process-ing @thePeras @bdmendes
Update uni/lib/controller/local_storage/app_references_database.dart
190bf34 
Co-authored-by: João Pereira <53405284+thePeras@users.noreply.github.com>
@Process-ing @bdmendes
Changed helper text color to default.
4236b3c
@Process-ing @bdmendes
Update AppReferencesDatabase description.
c288c8f
@Process-ing @bdmendes
Change references placeholder text for no references.
ec8702c
@bdmendes bdmendes force-pushed the feature/scrap_references branch from da51a2c to ec8702c Compare July 10, 2023 15:40
@bdmendes bdmendes merged commit eee7309 into develop Jul 10, 2023
@bdmendes bdmendes deleted the feature/scrap_references branch July 10, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thePeras thePeras thePeras approved these changes

@bdmendes bdmendes bdmendes approved these changes

@brunogomes30 brunogomes30 Awaiting requested review from brunogomes30

Assignees

@Process-ing Process-ing

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Scrap MB references from Sigarra
4 participants
@Process-ing @thePeras @brunogomes30 @bdmendes