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

Migrate CSS for page-list component #407

Closed
SYU15 opened this issue Sep 11, 2019 · 3 comments · Fixed by #414
Closed

Migrate CSS for page-list component #407

SYU15 opened this issue Sep 11, 2019 · 3 comments · Fixed by #414

Comments

@SYU15
Copy link
Contributor

SYU15 commented Sep 11, 2019

Right now the CSS for this component lives in styles.css:
https://github.com/edgi-govdata-archiving/web-monitoring-ui/blob/master/src/css/styles.css#L130

This is vanilla CSS that is currently loaded at the global level but we are currently migrating our CSS to CSS Modules.

Here is an example PR where the source-info component was migrated over.

To accomplish this we need to do the following:

  1. Add the CSS to the new pipeline by migrating it out of styles.css and into a new file called page-list.css. The directory structure should look like this:
src/components
└─┬ page-list
  ├── page-list.jsx
  └── page-list.css
  1. In page-list.jsx, the new CSS file needs to imported:
import './page-list.css';
  1. The .page-list tbody tr and any other selectors using more than one element/class name/etc., need to be refactored to be a single class name. Something like .page-list-row, for example, could work or something similar. These classes need to be added/replaced in the page-list.jsx file. Feel free to ask if you aren't sure where the new classes should go in the markup.

  2. In page-list.jsx, the className tags needs to be added or replaced for styleName for the component level styles only. Global styles should stay in className tags and a component can have both className and styleName tags at the same time if necessary. Feel free to ask if you aren't sure if a style falls under either category. (This basically determines which classes get suffixed with a hash to make sure it is scoped at the component level).

  3. Fix any tests that fail because you refactored the class names. You can run the tests using npm test.

Here is an example of styleName vs className being used in our codebase:

<a
styleName="source-info-link"
href={this.props.to.source_metadata.view_url}
target="_blank"
rel="noopener noreferrer"
>
{sourceTypeName[this.props.to.source_type]} next page version
<i className="fa fa-arrow-right fa-right-icon fa-no-hover" aria-hidden="true" />
</a>

@Mr0grog Mr0grog added this to Inbox in Web Monitoring via automation Sep 26, 2019
@Mr0grog Mr0grog moved this from Inbox to Ready in Web Monitoring Sep 26, 2019
@jatinAroraGit
Copy link
Contributor

Hi there,
I would like to work on this issue.
Can it be assigned to me please.
Thanks.

@Mr0grog
Copy link
Member

Mr0grog commented Sep 27, 2019

That’s great! Would you please work through and complete #411 first, though?

@jatinAroraGit
Copy link
Contributor

Sure @Mr0grog , on it.

jatinAroraGit pushed a commit to jatinAroraGit/web-monitoring-ui that referenced this issue Oct 20, 2019
Web Monitoring automation moved this from Ready to Done! Nov 4, 2019
Mr0grog pushed a commit that referenced this issue Nov 4, 2019
This switches the page-list component from being a single `.jsx` file to being a directory with `.jsx` and `.css` files. Fixes #407.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Web Monitoring
  
Done!
Development

Successfully merging a pull request may close this issue.

3 participants