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

Include duration in event detail view #227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andreaswolf
Copy link

@andreaswolf andreaswolf commented Dec 30, 2019

Description

Includes the event duration after the start time in the detail view's top bar

Before

No duration was shown

After

The duration (e.g. "60 min") is shown after the start time.

Related

@johnjohndoe johnjohndoe added the Feature request A request for a new feature. label Dec 30, 2019
@johnjohndoe
Copy link
Member

@andreaswolf Thank you for your contribution. This feature is closely related to a recent change in the app which you can find in pull request #192 and issue #10.

I would like to raise the question if a visual separation of date/time and duration makes sense here. As a separator a well chosen icon might even help with that. In that case you would have to create a new view for the duration instead of concatenating the date/time and duration texts. - What do you think about this?

Additional, please provide before and after screenshots to ease the review process and to have the visual state of the app being documented.

Please check the "extreme" values such as a very short duration such as 5min and very long session such as 420min.

@johnjohndoe johnjohndoe self-assigned this Dec 30, 2019
@andreaswolf
Copy link
Author

@johnjohndoe Thanks for the feedback.

I also thought about this separation, I just wanted to keep the effort as low as possible for now – but it definitely makes sense to separate it.
I would also suggest to then use a "calendar" icon for the date/time field and the current icon for the duration instead.

I'll add screenshots once the changes are done.

@johnjohndoe
Copy link
Member

I haven't though about a good icon yet. We should discuss this up front. Research what other apps use might be helpful. Please keep in mind that the calendar icon is already used for the "add to calendar" action and therefore using it in two contexts might be confusing.

@johnjohndoe
Copy link
Member

@andreaswolf Please join the discussion in #231. I also added the issue to your pull request description.

@johnjohndoe johnjohndoe added the Date & time Processing temporal data (parsing, formatting, testing ...) label Feb 27, 2020
@johnjohndoe johnjohndoe added the Session details The screen with all information about the session. label Nov 2, 2021
@bitPogo
Copy link
Collaborator

bitPogo commented Nov 19, 2021

Is that ready to be reviewed? If so, could you please resolve the conflict.

@johnjohndoe
Copy link
Member

The conversion here stopped at the end of 2019 as you can see above. In my opinion the PR is not ready. We still need to discuss how the visual change should look in the end. Not sure if @andreaswolf wants to continue here.

@johnjohndoe johnjohndoe added the Stale Inactive or abandoned issue or pull request with no recent activity. label Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Date & time Processing temporal data (parsing, formatting, testing ...) Feature request A request for a new feature. Session details The screen with all information about the session. Stale Inactive or abandoned issue or pull request with no recent activity.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants