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(clayui.com): Update content of some components #3226

Merged
merged 1 commit into from
May 20, 2020

Conversation

kresimir-coko
Copy link
Member

@kresimir-coko kresimir-coko commented May 14, 2020

Addresses some components listed in #3210

@bryceosterhaus I decided to send this halfway through to make it easier to review and iterate over. I'll be working on the other half of the components listed in the issue next.

Here's what I've done:

  • Add content where it was missing
  • Rename badges to badge
  • Rename text-input-localizable to localized-input
  • Update headings to start at h1 (amount of # in Markdown)
  • Update some navigations where it was needed

Components included

  • Badges
  • Breadcrumbs
  • Dual List Box
  • Loading Indicator
  • Localized Input
  • Multi Step Nav

Something I noticed

Localized Input is missing prop to render textarea, this example was included in the Markup/CSS page but we had no support for that. So I removed the example with the idea of eventually adding it back when we add a prop to the component.

@coveralls
Copy link

coveralls commented May 14, 2020

Pull Request Test Coverage Report for Build 110175154

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 78.577%

Totals Coverage Status
Change from base Build 110132480: 0.0%
Covered Lines: 2308
Relevant Lines: 2738

💛 - Coveralls

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 14, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c4ad126:

Sandbox Source
goofy-field-k21wm Configuration
elated-dhawan-lhwgk Configuration

Copy link
Member

@bryceosterhaus bryceosterhaus left a comment

Choose a reason for hiding this comment

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

See my comment at #3224 (review) regarding h1 tags in our docs.

I also noticed there are quite a few redundancies from the "description" frontmatter in most of these files. I'm not sure if we accidentally removed "description" from being used, but in the past each page was generated with the first block of content being the description from the frontmatter.

Additionally, I noticed you were going through all the css docs as well. I think for now, its best that we not worry about those at the same time as the react docs. Lets iterate first through react and then if needed, we can go through the css docs w/ @pat270. So for this PR and additional PRs, lets not worry about the css docs right now.

@kresimir-coko
Copy link
Member Author

I've pushed a commit containing the suggestions, also some notes:

  • I have discarded the changes to CSS page files as you mentioned
  • The descriptions in the frontmatter of MDX files aren't being rendered in React pages, but in CSS pages, which is why I've included them in the content

Static files keep getting modified

It's been happening for a while now, like a month or even 2, but I keep getting files in static folder appear in my git diff when I run any of the yarn build scripts (i.e. yarn develop in clayui.com folder). Files in question, as well as the files in question the _global-functions.scss file gets modified when running yarn build in root.
Is there something I can do about this? Am I missing any configuration, I don't remember changing anything when this started happening.

Copy link
Member

@bryceosterhaus bryceosterhaus left a comment

Choose a reason for hiding this comment

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

Hey @kresimir-coko, your comment gave me an idea I think we can start with first before actually updating the docs. See my comment below about establishing a grid of questions that we can use to layout our docs.

@kresimir-coko
Copy link
Member Author

kresimir-coko commented May 19, 2020

Hey @bryceosterhaus I've made some more changes to this as taken from the discussion. Not much to it, I've removed some content that I've added when I thought we needed more of it. Also, I've done the changes to frontmatter.description so that they get rendered on both pages as a subheading

Hmm, for some reason CI seems to be breaking, but locally everything looks fine.

Copy link
Member

@bryceosterhaus bryceosterhaus left a comment

Choose a reason for hiding this comment

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

@kresimir-coko can you check out the failing build? Other than that, it looks good to me

@pat270
Copy link
Member

pat270 commented May 19, 2020

@kresimir-coko for _global-functions.scss I think yarn format is modifying the SVG code in the generated $lx-icons map during release? Clay CSS build is rounding values 0-32-14.336-32-32s14.299-32 to 0-32-14.336-32-32s14.3-32 (.299-32 to .3-32).

When Clay CSS generates $lx-icons it just copies the SVG over from the image directory, disabling formatting there will fix this issue.

… Breadcrumb, Dual List Box, Loading Indicator, Localized Input & Multi Step Nav
@bryceosterhaus
Copy link
Member

LGTM!

@bryceosterhaus bryceosterhaus merged commit aae1f77 into liferay:master May 20, 2020
@kresimir-coko kresimir-coko deleted the 3210 branch June 1, 2020 09:38
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