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

fix: enhance session card in schedule page #5809

Merged

Conversation

sachinchauhan2889
Copy link
Contributor

@sachinchauhan2889 sachinchauhan2889 commented Nov 28, 2020

Fixes #5787
Fixes #5797

Short description of what this resolves:

screenshots

session card

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)

@auto-label auto-label bot added the fix label Nov 28, 2020
@vercel
Copy link

vercel bot commented Nov 28, 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/bpttboyrj
✅ Preview: https://open-event-frontend-git-enhance-session-card.eventyay.vercel.app

@iamareebjamal
Copy link
Member

Right indent has not changed
image

And even if line breaks, it should not cross below the icon

@iamareebjamal
Copy link
Member

image

If we move the speaker position below speaker name, it will look better and break line less often

@iamareebjamal
Copy link
Member

Make it smaller and less highlighted than the name

@sachinchauhan2889
Copy link
Contributor Author

new issue sachin

@codecov
Copy link

codecov bot commented Nov 28, 2020

Codecov Report

Merging #5809 (22bd4d2) into development (7995611) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #5809   +/-   ##
============================================
  Coverage        23.37%   23.37%           
============================================
  Files              511      511           
  Lines             5472     5472           
  Branches            65       65           
============================================
  Hits              1279     1279           
  Misses            4176     4176           
  Partials            17       17           

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 7995611...22bd4d2. Read the comment docs.

@iamareebjamal
Copy link
Member

Change the opacity of speaker designation (below the name) to 0.8 and font size to .9rem

Also, since names are not grouped now, remove the paranthesis

@iamareebjamal
Copy link
Member

Also add ml-2 to speaker name and designation div

@iamareebjamal
Copy link
Member

image

For this, create a max-width style so that designation wraps to next line and images align vertically

Copy link
Member

@divyamtayal divyamtayal left a comment

Choose a reason for hiding this comment

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

Does not work on small screens (eg iPad (pro/mini) and others)
Screen Shot 2020-11-30 at 13 47 47
Is not properly aligned
Screen Shot 2020-11-30 at 13 47 08

@mariobehling
Copy link
Member

The comma issue has been solved in another PR now. Please resolve conflicts.

@iamareebjamal
Copy link
Member

@sachinchauhan2889 @daretobedifferent18 We want this to be part of 1.19.0 release, which is blocked due to this, so, tomorrow, please work together to iron out the few layout issues as soon as possible so we can release 1.19.0

Thanks

@sachinchauhan2889
Copy link
Contributor Author

@iamareebjamal sir, please review

<i class="icon {{if this.slidesUploaded 'download' 'linkify'}}"></i>
{{if this.slidesUploaded (t 'Download Slides') (t 'Link to Slides')}}
</button>
<div style="white-space: nowrap;">
Copy link
Member

Choose a reason for hiding this comment

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

Why wrap in this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when title is long it break into another line and there is a break down in button text also.
button

to avoid this i wrap this button and put style="white-space: nowrap;" so that there is no line break in button text when title is long.

<i class="icon video"></i>
{{t 'Join Video'}}
</button>
<div style="white-space: nowrap;">
Copy link
Member

Choose a reason for hiding this comment

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

Why wrap in this?

@mariobehling
Copy link
Member

Merging branch to see if that fixes it.

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.

Button is there after branch merge. Thank you

@mariobehling mariobehling merged commit 02db3b9 into fossasia:development Dec 4, 2020
@sachinchauhan2889 sachinchauhan2889 deleted the enhance-session-card branch December 5, 2020 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public Schedule: Enhance Speaker Display Public Schedule: Enhance Session Tile Area
4 participants