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

enh: Adding verticle timeline to session page #4921

Closed
wants to merge 13 commits into from
Closed

enh: Adding verticle timeline to session page #4921

wants to merge 13 commits into from

Conversation

maze-runnar
Copy link
Contributor

Fixes #
Screenshot from 2020-08-31 22-43-56

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@vercel vercel bot temporarily deployed to Preview August 31, 2020 17:14 Inactive
@vercel
Copy link

vercel bot commented Aug 31, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/4tcknvddr
✅ Preview: https://open-event-frontend-git-fork-maze-runnar-patch-7.eventyay.vercel.app

@vercel vercel bot temporarily deployed to Preview August 31, 2020 17:16 Inactive
@iamareebjamal
Copy link
Member

iamareebjamal commented Sep 1, 2020

There shouldn't be any line breaks in time. See the example UI in meeting notes

@maze-runnar
Copy link
Contributor Author

Screenshot from 2020-09-01 20-42-48

@iamareebjamal
Copy link
Member

Now it's 12 hour time without AM/PM

@maze-runnar
Copy link
Contributor Author

@iamareebjamal now it's showing 24 hour format, similar to website generate;
Screenshot from 2020-09-01 22-36-33

@iamareebjamal
Copy link
Member

@maze-runnar The build is failing. Please finalize old issues before taking on new ones

@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #4921 into development will decrease coverage by 0.50%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #4921      +/-   ##
===============================================
- Coverage        23.22%   22.72%   -0.51%     
===============================================
  Files              481      485       +4     
  Lines             5098     5153      +55     
  Branches            18       21       +3     
===============================================
- Hits              1184     1171      -13     
- Misses            3910     3978      +68     
  Partials             4        4              
Impacted Files Coverage Δ
app/controllers/index.js 36.36% <0.00%> (-54.55%) ⬇️
app/utils/computed-helpers.js 22.50% <0.00%> (-7.50%) ⬇️
app/components/forms/wizard/basic-details-step.js 18.57% <0.00%> (-5.82%) ⬇️
app/components/widgets/forms/link-input.js 42.10% <0.00%> (-5.27%) ⬇️
app/components/forms/wizard/custom-form-input.ts 4.16% <0.00%> (-4.17%) ⬇️
app/components/schedule.ts 62.22% <0.00%> (-3.64%) ⬇️
app/routes/explore.js 58.62% <0.00%> (-0.64%) ⬇️
app/router.js 100.00% <0.00%> (ø)
app/mixins/event-wizard.js 1.13% <0.00%> (ø)
app/helpers/ui-grid-number.js 34.78% <0.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a93c224...aa69e8e. Read the comment docs.

@maze-runnar
Copy link
Contributor Author

@maze-runnar The build is failing. Please finalize old issues before taking on new ones

Done

@iamareebjamal
Copy link
Member

Thanks. I'll review it

{{#each this.model.session as |session|}}
<Public::SessionItem @session={{session}} @timezone={{this.timezone}} />
<li class="tl-item" ng-repeat="item in retailer_history">
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iamareebjamal I have removed unrequired code from css and hbs both.

@vercel vercel bot temporarily deployed to Preview September 7, 2020 09:33 Inactive
@mariobehling
Copy link
Member

We can improve the design at a later stage in detail, but for now at least the times should follow the spacing as the above design items. Please move the time to the right in order to follow an imaginary line from top to bottom (based on the headings above). Thanks.

91869617-bc329280-ec93-11ea-9293-92b979d685e9

@iamareebjamal
Copy link
Member

@maze-runnar Please center align the time so that it looks better

@vercel vercel bot temporarily deployed to Preview September 7, 2020 13:08 Inactive
@maze-runnar
Copy link
Contributor Author

Screenshot from 2020-09-07 18-38-58

@maze-runnar
Copy link
Contributor Author

Screenshot from 2020-09-07 18-42-11

@iamareebjamal
Copy link
Member

Nothing has changed. Please reread the comments and picture

@iamareebjamal
Copy link
Member

at least the times should follow the spacing as the above design items. Please move the time to the right in order to follow an imaginary line from top to bottom (based on the headings above). Thanks.

91869617-bc329280-ec93-11ea-9293-92b979d685e9

@iamareebjamal
Copy link
Member

The time now seems even more left aligned. Even going as far as being aligned to the sidebar

@maze-runnar
Copy link
Contributor Author

Nothing has changed. Please reread the comments and picture

the line should be in left & time should be in right?

@iamareebjamal
Copy link
Member

Time should not be aligned to the left. See mario's image

@maze-runnar
Copy link
Contributor Author

Time should not be aligned to the left. See mario's image

I am not exactly getting his image, what is trying to say.

@iamareebjamal
Copy link
Member

The time is aligned to the left. It should move to the right

@iamareebjamal
Copy link
Member

Not aligned to the right. It should move slightly to the right to match the margin on the rest of the content

@iamareebjamal
Copy link
Member

@snitin315 Can you please explain if you understand?

@maze-runnar
Copy link
Contributor Author

Not aligned to the right. It should move slightly to the right to match the margin on the rest of the content

ok, got it

@snitin315
Copy link
Member

Expected-

    Session
    10-11
    11-12

Current-

    Session
  10-11
  11-12

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 7, 2020

I have deleted the fork due to some issue, and fork again the project, so creating a new PR for this with updated changes. #5002

@maze-runnar maze-runnar closed this Sep 7, 2020
@maze-runnar maze-runnar mentioned this pull request Sep 7, 2020
5 tasks
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.

4 participants