Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

WIP - React Components Show React Logo#4728

Merged
jasonLaster merged 5 commits into
firefox-devtools:masterfrom
James300:react-component-tab-logos
Nov 18, 2017
Merged

WIP - React Components Show React Logo#4728
jasonLaster merged 5 commits into
firefox-devtools:masterfrom
James300:react-component-tab-logos

Conversation

@James300

@James300 James300 commented Nov 18, 2017

Copy link
Copy Markdown
Contributor

Associated Issue: #4151

Summary of Changes

  • Add React icon to React components tabs

Screenshots

react-icon-tab-2
react-icon-tab

The icon seems a little small, thoughts?

Comment thread src/components/Editor/Tabs.js Outdated
activeSearch: getActiveSearch(state),
searchOn: getActiveSearch(state) === "source"
searchOn: getActiveSearch(state) === "source",
sourceMetaData: getSourceMetaData(state, 0)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jasonLaster I was actually having a problem with how to fetch the sourceId for getSourceMetaData

Comment thread src/components/Editor/Tabs.js Outdated
activeSearch: getActiveSearch(state),
searchOn: getActiveSearch(state) === "source"
searchOn: getActiveSearch(state) === "source",
sourceMetaData: { isReactComponent: true }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hard-coded for now so I could complete the rest of the task, what should go here because I can't figure it out? 😢

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@James300 You can do this

@@ -484,13 +484,15 @@ class SourceTabs extends PureComponent<Props, State> {

 export default connect(
   state => {
+    const selectedSource = getSelectedSource(state);
+    const sourceId = selectedSource ? selectedSource.get("id") : "";
     return {
-      selectedSource: getSelectedSource(state),
+      selectedSource,
       searchTabs: getSearchTabs(state),
       sourceTabs: getSourcesForTabs(state),
       activeSearch: getActiveSearch(state),
       searchOn: getActiveSearch(state) === "source",
-      sourceMetaData: { isReactComponent: true }
+      sourceMetaData: getSourceMetaData(state, sourceId)
     };
   },
   dispatch => bindActionCreators(actions, dispatch)

I think there is still the work to set the sourceMetaData in redux

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank Bomsy this did it 👍

@James300

Copy link
Copy Markdown
Contributor Author

I noticed a few problems with the framework isReactComponent:

  • React.Component does not get recognized as a component
  • Using require instead of import does not work

Also, the React Icon does not show when reloading previously opened files.

Comment thread src/components/Editor/Tabs.css Outdated
padding-top: 12px;
height: 14px;
width: 14px;
background: var(--theme-highlight-blue);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dark mode might benefit from a different color... something darker...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image
image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is using bluegrey, thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i like it

@jasonLaster jasonLaster left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@jasonLaster jasonLaster merged commit b748028 into firefox-devtools:master Nov 18, 2017
@jasonLaster jasonLaster mentioned this pull request Nov 23, 2017
7 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants