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

feat: Enhance session item UI #5819

Merged
merged 10 commits into from
Nov 30, 2020
Merged

feat: Enhance session item UI #5819

merged 10 commits into from
Nov 30, 2020

Conversation

divyamtayal
Copy link
Member

@divyamtayal divyamtayal commented Nov 29, 2020

Fixes #5817
Fixes #5774
Fixes #5818

Short description of what this resolves:

Changes proposed in this pull request:

  • When session is expanded hide profile image, name, designation and org on the top area
  • Leave session info on top area as is
  • On the expanded area show position and organization below the speaker name without brackets
  • If the speaker uploaded slides show a button "Download Slides". Also include an icon in the download button.
  • If the speaker has only linked to the slides show a button "Link to Slides". Also include a link icon as part of the button.
  • On the schedule a comma and space show up even though they are not required. This happens if the speaker has not filled in a position. Expected no comma and space should show up if the speaker has not filled in a position. Equally if the speaker has not filled in a company no comma and space should show up.

ScreenShots

Screenshot from 2020-11-29 11-48-53
Screenshot from 2020-11-29 11-49-26

Screenshot from 2020-11-29 15-05-39

Screenshot from 2020-11-29 17-41-37

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
Copy link

vercel bot commented Nov 29, 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/aptw8xoue
✅ Preview: https://open-event-frontend-git-session-expanded-5817.eventyay.vercel.app

@divyamtayal divyamtayal changed the title Hide Profile Public Schedule: Expanded session info - add position and organization, hide top profile icon and details on top when expanded Nov 29, 2020
@divyamtayal
Copy link
Member Author

@iamareebjamal It's Done.

@codecov
Copy link

codecov bot commented Nov 29, 2020

Codecov Report

Merging #5819 (e75fadf) into development (a5104cc) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5819      +/-   ##
===============================================
- Coverage        23.46%   23.38%   -0.08%     
===============================================
  Files              513      513              
  Lines             5477     5481       +4     
  Branches            63       63              
===============================================
- Hits              1285     1282       -3     
- Misses            4176     4183       +7     
  Partials            16       16              
Impacted Files Coverage Δ
app/components/public/session-item.js 14.28% <0.00%> (-3.90%) ⬇️
app/models/speaker.js 0.00% <0.00%> (ø)
app/components/tabbed-navigation.js 33.33% <0.00%> (-20.00%) ⬇️

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 a5104cc...e75fadf. Read the comment docs.

Copy link
Contributor

@sachinchauhan2889 sachinchauhan2889 left a comment

Choose a reason for hiding this comment

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

microlocation and date-time information shift to left side after expanding session card. They must remain on right side even on expanding session.

@divyamtayal
Copy link
Member Author

microlocation and date-time information shift to left side after expanding session card. They must remain on right side even on expanding session.

That was actually a bug before which was not working but now its working correctly

@iamareebjamal
Copy link
Member

#5774 is not fixed

@divyamtayal
Copy link
Member Author

#5774 is not fixed

Working on both #5774 #5818
See the Checklist in the description

@divyamtayal
Copy link
Member Author

@iamareebjamal only left with #5818 but in this issue actually the link is been added in short Abstract and not in Slide url, so it will remain as it is, I think we can't change that.
Pls let me know what to do

@divyamtayal
Copy link
Member Author

@iamareebjamal Done.

@lgtm-com
Copy link

lgtm-com bot commented Nov 29, 2020

This pull request introduces 2 alerts when merging 160225f into a5104cc - view on LGTM.com

new alerts:

  • 2 for Incomplete URL substring sanitization

mariobehling
mariobehling previously approved these changes Nov 29, 2020
Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

Nice work! Thanks

@mariobehling
Copy link
Member

Oh, in the last iteration the date and time now appear twice.

Compare https://open-event-frontend-git-session-expanded-5817.eventyay.vercel.app/e/8fa7fd14/schedule

Screenshot from 2020-11-29 14-50-54

@divyamtayal
Copy link
Member Author

@iamareebjamal Made Changes

@lgtm-com
Copy link

lgtm-com bot commented Nov 29, 2020

This pull request introduces 2 alerts when merging b234454 into a5104cc - view on LGTM.com

new alerts:

  • 2 for Incomplete URL substring sanitization

@lgtm-com
Copy link

lgtm-com bot commented Nov 29, 2020

This pull request introduces 2 alerts when merging e75fadf into a5104cc - view on LGTM.com

new alerts:

  • 2 for Incomplete URL substring sanitization

@divyamtayal
Copy link
Member Author

@iamareebjamal Pls review. This PR is completed

@iamareebjamal iamareebjamal changed the title Public Schedule: Expanded session info - add position and organization, hide top profile icon and details on top when expanded feat: Enhance session item UI Nov 30, 2020
@auto-label auto-label bot added the feature label Nov 30, 2020
@iamareebjamal
Copy link
Member

iamareebjamal commented Nov 30, 2020

@mariobehling I am merging this, but session collapsed will look very cluttered when there is both video stream and slides attached to it

Also, we need to think if we still need to show the video link if the session has passed? Because there will be no one there then

@iamareebjamal iamareebjamal merged commit 2072e06 into fossasia:development Nov 30, 2020
@mariobehling
Copy link
Member

Yes, Thank you.

Let's talk about this in the weekly meeting.

@divyamtayal divyamtayal deleted the session-expanded-5817 branch December 10, 2020 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants