Skip to content
This repository was archived by the owner on Feb 29, 2020. It is now read-only.

style(content): #3745 Add per-platform css files#3784

Merged
k88hudson merged 1 commit intomozilla:masterfrom
k88hudson:gh3745
Nov 3, 2017
Merged

style(content): #3745 Add per-platform css files#3784
k88hudson merged 1 commit intomozilla:masterfrom
k88hudson:gh3745

Conversation

@k88hudson
Copy link
Copy Markdown
Contributor

Includes a windows-specific search focus border style

Comment thread system-addon/jar.mn Outdated
content/vendor/react-redux.js (./vendor/react-redux.js)
content/data/ (./data/*)
#ifdef XP_MACOSX
content/data/content/activity-stream.css (./data/content/activity-stream-mac.css)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The content/data/ (./data/*) line is going to package all of them, so most likely leading to unreferenced file test failure. Do we make a new "top level" directory outside of data? css/ ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose you can create a new directory in this PR, and I'll update it in #3755 if we don't want to put css in prerendered/static/activity-stream-mac.css.

Alternatively, the only thing inside data/ will be data/content/ after I remove locales.json from the other PR #3755. So we could have a data/css/*.css that we jar.mn package appropriately… and now that only activity-stream.bundle.js is the only single file:

  content/data/content/activity-stream.bundle.js (./data/content/activity-stream.bundle.js)
  content/data/content/assets/ (./data/content/assets/*)
  content/data/content/tippytop/ (./data/content/tippytop/*)
#ifdef …
  content/data/content/activity-stream.css (./data/css/activity-stream-mac.css)

And I suppose with that change, I could move prerendered to data/prerendered/.

(I suppose we could move things "up" a directory, i.e., ./data/{activity-stream.bundle.js,assets,css,tippytop,prerendered} if we're going to CLOBBER anyway although history might be broken. Separately, we could expose the resource://activity-stream/data/content/* as just resource://activity-stream/content/*…)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On a related note, should I be updating #3775 (wow #3775 = package all locales vs #3755 = remove locales.json) and the related mozreview patch to use resource://activity-stream/data/content/prerendered/locales/en-US instead of resource://activity-stream/prerendered/locales/en-US ? if we move things to ./data/prerendered ?

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.

I like the idea of moving stuff to top-level directories (content/data/content was kind of weird anyway)

@@ -0,0 +1,7 @@
/* This is the windows variant */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to be explicit, this comment block will appear in the generated file. Which I suppose would be handy for debugging?

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.

Yep, that's the idea


// Search border should only have a 1-pixel blue border, so no box-shadow is needed.
.search-wrapper input:focus {
box-shadow: none;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The change from #3785 removes the border, so we'll have to fix this a different way.

@k88hudson
Copy link
Copy Markdown
Contributor Author

Ok, I moved everything to a css directory and updated relevant paths. What do you think?

@k88hudson k88hudson requested a review from Mardak November 2, 2017 17:42
Copy link
Copy Markdown
Member

@Mardak Mardak left a comment

Choose a reason for hiding this comment

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

Looks fine. I guess I'll keep the generated prerendered files at system-addon/prerendered/ like the now system-addon/css/

Includes a windows-specific search focus border style
@k88hudson k88hudson assigned k88hudson and unassigned Mardak Nov 3, 2017
@k88hudson k88hudson merged commit 4557c3b into mozilla:master Nov 3, 2017
@as-pine-proxy
Copy link
Copy Markdown
Collaborator

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants