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(Table): Change column width to match style guidelines #850

Merged
merged 3 commits into from
Nov 7, 2019

Conversation

Artikodin
Copy link
Contributor

@Artikodin Artikodin commented Oct 31, 2019

  • Feature
  • Fix
  • Enhancement

Description

What is already done in that PR :

  • changed column width to respect the style guidelines
    How that works :
  • I check the column's number on the table.
  • Add one to the result (because first column take two column width)
  • Set column width except first one to 100% / column's number + 1
  • All cell gonna have the same width except the first one which take remaining space (2 columns width)
  • First column : To crop text and avoid text overflowing on the other cell I remove display: flex attribut which lead to an issue on safari :

Screen Shot 2019-10-31 at 16 02 15

icon is not aligned anymore

What is need to be done :

  • Fix icon alignement issue
  • Check on mobile (Sonar) if the preview implementation works
  • Remove freshchat logo
  • Don't fill height when table is not long enough
    Screenshot 2019-10-31 at 10 37 13 (1)
  • Fix/Add tests
  • Add shadows to the table
    I already started that point. The way I think the solution :
  • To add shadow effect I display a transparent element on to of table content which fit to cells body
  • In that element need to add 2 more elements (div or pseudo element) on each side width shadow in it
  • When table content is scrollY > 0 display left shadow
  • When table content is scrollY < max content width display right shadow

Related / Associated Jira Cards :

BP-434

Todo - Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.

@anglol
Copy link
Contributor

anglol commented Nov 6, 2019

I just added the shadows on scroll.

@spartDev spartDev added need review and removed need review WIP Work in progress labels Nov 6, 2019
@anglol anglol force-pushed the fix/table_max-width branch 2 times, most recently from 00a3ed5 to d7256ce Compare November 6, 2019 14:08
Copy link
Contributor

@sun-tea sun-tea left a comment

Choose a reason for hiding this comment

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

May not be directly related to the PR but I noticed that the height of the children's row is not the same (padding difference)
Screenshot 2019-11-06 at 16 34 59

src/Table/elements.js Outdated Show resolved Hide resolved
@anglol anglol force-pushed the fix/table_max-width branch 2 times, most recently from 94ece48 to af3e39f Compare November 6, 2019 16:54
@ArTiSTiX ArTiSTiX requested a review from sun-tea November 6, 2019 17:11
@sun-tea sun-tea merged commit 048aec1 into master Nov 7, 2019
@sun-tea sun-tea assigned anglol and unassigned spartDev and sun-tea Nov 7, 2019
@sun-tea sun-tea deleted the fix/table_max-width branch December 9, 2019 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants