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

Occurrences Widget - Start and End #22

Closed
Vanethos opened this issue Aug 14, 2019 · 19 comments · Fixed by #71
Closed

Occurrences Widget - Start and End #22

Vanethos opened this issue Aug 14, 2019 · 19 comments · Fixed by #71
Assignees
Labels
good first issue Good for newcomers hacktoberfest Hacktoberfest is here! Don't know which issues you should choose? Pick these!

Comments

@Vanethos
Copy link
Contributor

Vanethos commented Aug 14, 2019

Description
Separate widget for the "Start and End" of the Occurrences.

File Location

- presentation
|__ ui

Requirements

  • Widget that with the Occurrences object displays information about:
    • Start and end of the occurrence
    • Last updated

UI

imagem

NOTES
Until the API Is completed, assume that this widget has as an input 3 DateTime objects.

@OldMetalmind OldMetalmind added this to the v1.0.0 - Akita milestone Aug 16, 2019
@OldMetalmind OldMetalmind added the good first issue Good for newcomers label Aug 24, 2019
@Vanethos Vanethos assigned Vanethos and unassigned Vanethos Sep 20, 2019
@Vanethos Vanethos added the hacktoberfest Hacktoberfest is here! Don't know which issues you should choose? Pick these! label Sep 20, 2019
@TomerPacific
Copy link
Contributor

@Vanethos , is this issue still up for grabs?

@OldMetalmind
Copy link
Member

Hello @TomerPacific it still is. Thumbs up 👍 this message and I'll assign it to you.

@TomerPacific
Copy link
Contributor

TomerPacific commented Oct 5, 2019

@OldMetalmind , is there anything specific I should be aware of? Do you want to separate it into two widgets? Naming conventions? Stateless/Stateful?

@Vanethos
Copy link
Contributor Author

Vanethos commented Oct 6, 2019

@TomerPacific

This Widget shpuld be Stateless.

Since there are no other widgets yet for this project, I'd name it as OccurrencesTimeWidget, or anything that you feel might be proper for this widget.

This can be one widget only

Anything else, we'll be here to help you!

@TomerPacific
Copy link
Contributor

@Vanethos , I have built a rough draft of the widget, but when trying to run the application, I get the following error:

Compiler message:
../Flutter%20SDK/flutter/.pub-cache/hosted/pub.dartlang.org/flare_flutter-1.5.7/lib/flare.dart:601:14: Error: The method 'FlutterActor.makeShapeNode' has fewer positional arguments than those of overridden method 'Actor.makeShapeNode'.
ActorShape makeShapeNode() {
^
../Flutter%20SDK/flutter/.pub-cache/hosted/pub.dartlang.org/flare_dart-1.4.8/lib/actor.dart:68:14: Context: This is the overridden method ('makeShapeNode').
ActorShape makeShapeNode(ActorShape source) {
^
Compiler failed on C:\Users\Tomer\Documents\mobile-app\lib\main.dart

FAILURE: Build failed with an exception.

  • Where:
    Script 'C:\Users\Tomer\Documents\Flutter SDK\flutter\packages\flutter_tools\gradle\flutter.gradle' line: 765
  • What went wrong:
    Execution failed for task ':app:compileFlutterBuildDebugX86'.

Process 'command 'C:\Users\Tomer\Documents\Flutter SDK\flutter\bin\flutter.bat'' finished with non-zero exit value 1

Obviously, I have not touched the Flare library and wanted to know if you have encountered this error. I did notice there is a newer version out there, but didn't want to upgrade without your consideration (might fix the issue).

@Vanethos
Copy link
Contributor Author

Vanethos commented Oct 8, 2019

@TomerPacific , a couple of questions:

  • Are you on branch develop?
  • Can you put here the output of flutter doctor?

Because on our end the app is running, in fact in each commit Travis tries to build the app, so supposedily it should be working in the latest stable branch of Flutter (I also tested building the app now and it worked)

@TomerPacific
Copy link
Contributor

TomerPacific commented Oct 8, 2019

@Vanethos ,
I have branched out from develop into feature/Occurrences-Widget-22.

Running flutter doctor yields:

$ flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[√] Flutter (Channel stable, v1.9.1+hotfix.4, on Microsoft Windows [Version 10.0.18362.356], locale en-IL)
[!] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
! Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses
[√] Android Studio (version 3.5)
[!] Connected device
! No devices available

! Doctor found issues in 2 categories.

I'm going to try returning to develop and running the application from there.

UPDATE
switched back to develop/imported the project again to Android Studio and got the same results

@Vanethos
Copy link
Contributor Author

Vanethos commented Oct 8, 2019

A quick question, if you update Flare's, can you run the app?

If so, and with no problem, please create a new PR just with that update, I'll approve it and you can then continue working on this.

Sorry for this experience btw! But on my end I'm not having any issues so it's hard to help and debug it :(

@TomerPacific
Copy link
Contributor

@Vanethos ,
I have applied what you have suggested and the application runs now.
I have upgraded the Flare package to the latest version, 1.5.15, which was released yesterday.

Do you want me to open an issue and then a pull request to reference it?

The issue might originate from my flutter version?

Flutter 1.9.1+hotfix.4 • channel stable • https://github.com/flutter/flutter.git Framework • revision cc949a8e8b (12 days ago) • 2019-09-27 15:04:59 -0700 Engine • revision b863200c37 Tools • Dart 2.5.0

@Vanethos
Copy link
Contributor Author

Vanethos commented Oct 9, 2019

@TomerPacific if you could open the issue and send the PR that would be awesome 🙏

@TomerPacific
Copy link
Contributor

@Vanethos , an issue has been opened and a PR has been made.

@Vanethos
Copy link
Contributor Author

Vanethos commented Oct 9, 2019

@TomerPacific done!

@TomerPacific
Copy link
Contributor

@Vanethos ,

this is what I have come up with :

vostro

I have a few questions before opening a PR:

  1. The text on the bottom row has been downscaled to fit the width of the container. Instead of entering a scale which may not fit all screens, do you allow the use of the auto size text library?
  2. The layout outline of the widget is as follows:
Material 
  |
Container
 |
Column
 |     |
Row   Row
 |
Column

Would you have preferred a different layout for this widget?
3. The widget takes three DateTime parameters for start, end and lastUpdated time
4. I have tested this page by routing the about page in the menu to open the widget.

@Vanethos
Copy link
Contributor Author

@TomerPacific

  • You still need to put inside an element like a card, just as it is in the original post.
  • As of the autosize library, it is best to open a different issue and a PR just for that.
  • What happens if one of the date-time elements, end for example, is null?

Other than that, I have to say I was fairly impressed with such detailed and informative comment. This is great quality content for an open-source project. Great work 👏

@TomerPacific
Copy link
Contributor

@Vanethos ,
thanks for the feedback.
In response to what you wrote:

  • There is no mention of a Card element (or at least I didn't understand there was a requirement for one), so I just want to make sure i understand. You would like the hierarchy to contain a card element that the dates will be displayed on?

  • If you request a different issue and PR for the autosize library, do you prefer that I leave things as they are regarding the size of the text?

  • Your third question is a great one, since I have already implemented code (when I posted my comment) to tackle this issue. Meaning, that if the dateTime object is null, then all that will display is "--" (per the example).

If there is anything more that is required, please let me know.

@Vanethos
Copy link
Contributor Author

@TomerPacific

  • When I say "card" it can be a container with a background! As you can see from the image, it has a rectangle all around it with some drop shadow

  • Yes, leave things as they are!

  • Yep that is what it is intended!

@TomerPacific
Copy link
Contributor

@Vanethos ,
I have opened a PR.

@Vanethos
Copy link
Contributor Author

Hey @TomerPacific ! I've commented the PR and requested changes!

Great work so far!

@TomerPacific
Copy link
Contributor

@Vanethos , I have reviewed all your comments and updated the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest Hacktoberfest is here! Don't know which issues you should choose? Pick these!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants