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

Table sidebar styling #196

Merged
merged 12 commits into from
Mar 24, 2020
Merged

Table sidebar styling #196

merged 12 commits into from
Mar 24, 2020

Conversation

Joozty
Copy link
Member

@Joozty Joozty commented Mar 19, 2020

No description provided.

@Joozty Joozty self-assigned this Mar 19, 2020
@vercel
Copy link

vercel bot commented Mar 19, 2020

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

🔍 Inspect: https://zeit.co/oacore/dashboard/g9djbh2ea
✅ Preview: https://dashboard-git-table-button.oacore.now.sh

@Joozty Joozty mentioned this pull request Mar 19, 2020
@viktor-yakubiv viktor-yakubiv force-pushed the table branch 2 times, most recently from 9cf0339 to ace86da Compare March 23, 2020 16:03
@Joozty Joozty changed the base branch from table to master March 23, 2020 18:16
@Joozty Joozty marked this pull request as ready for review March 23, 2020 19:30
@Joozty Joozty requested a review from viktor-yakubiv March 23, 2020 19:30
Copy link
Member

@viktor-yakubiv viktor-yakubiv left a comment

Choose a reason for hiding this comment

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

The Details window header should be red if full text is not available.

Is it desired behaviour?

Details footer is in the bottom

Copy link
Member

@viktor-yakubiv viktor-yakubiv left a comment

Choose a reason for hiding this comment

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

The code looks okay to me. Please take a look at the behaviour I mentioned before. It should stay in the bottom but it can be improved later if you wish. Thank you for contributing 👍

Comment on lines -8 to -9
// We want to pass referrer tag to our website
// eslint-disable-next-line react/jsx-no-target-blank
Copy link
Member

Choose a reason for hiding this comment

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

Oh, interesting issue. I see we have to add Button to the list of components that the rule has to check. This comment should stay.

Copy link
Member Author

Choose a reason for hiding this comment

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

is it possible to configure it?

Copy link
Member

Choose a reason for hiding this comment

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

Check the custom link components option.

Also, we may want to enable allow-referrer option.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's quite clever 👍

components/table/Sidebar.jsx Show resolved Hide resolved
design/detail-list/index.jsx Show resolved Hide resolved
design/detail-list/index.jsx Show resolved Hide resolved
store/works.js Outdated Show resolved Hide resolved
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.

2 participants