Skip to content

Embroider wizardry #10

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

Open
wants to merge 470 commits into
base: classic-wizardry-components
Choose a base branch
from

Conversation

chancancode
Copy link
Owner

No description provided.

@chancancode chancancode changed the base branch from classic-wizardry-templates to classic-wizardry-components September 27, 2023 01:16
@chancancode chancancode force-pushed the classic-wizardry-components branch from f82e7e0 to 3fef4f6 Compare October 4, 2023 04:10
@chancancode chancancode force-pushed the classic-wizardry-components branch 2 times, most recently from 893a2a9 to e7026b4 Compare October 19, 2023 23:28
@chancancode chancancode force-pushed the classic-wizardry-components branch from e7026b4 to 3a9dab6 Compare October 20, 2023 00:17
@chancancode chancancode force-pushed the classic-wizardry-components branch from 3a9dab6 to b5ec3b1 Compare October 20, 2023 04:04
dependabot bot and others added 20 commits November 7, 2023 22:53
Bumps [net-protocol](https://github.com/ruby/net-protocol) from 0.2.1 to 0.2.2.
- [Release notes](https://github.com/ruby/net-protocol/releases)
- [Commits](ruby/net-protocol@v0.2.1...v0.2.2)

---
updated-dependencies:
- dependency-name: net-protocol
  dependency-type: indirect
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [cgi](https://github.com/ruby/cgi) from 0.3.6 to 0.4.0.
- [Release notes](https://github.com/ruby/cgi/releases)
- [Commits](ruby/cgi@v0.3.6...v0.4.0)

---
updated-dependencies:
- dependency-name: cgi
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [base64](https://github.com/ruby/base64) from 0.1.1 to 0.2.0.
- [Release notes](https://github.com/ruby/base64/releases)
- [Commits](ruby/base64@v0.1.1...v0.2.0)

---
updated-dependencies:
- dependency-name: base64
  dependency-type: indirect
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [date](https://github.com/ruby/date) from 3.3.3 to 3.3.4.
- [Release notes](https://github.com/ruby/date/releases)
- [Commits](ruby/date@v3.3.3...v3.3.4)

---
updated-dependencies:
- dependency-name: date
  dependency-type: indirect
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This commit adds an /admin/customize/theme-components route,
that opens the theme page with the components tab pre-selected,
so people can navigate to that directly.
…iscourse#24261)

Followup to 545e920

This commit fixes an issue where hashtags on user activity stream
items past page 1 did not get decorated. This is because of a bug
in the user stream component, where it was trying to get stream
items to decorate after the AJAX call but before they had been
rendered by Ember. This can be fixed by wrapping this decoration
logic in later() to run on the next runloop.
This removes all trivial usages of the `{{action}}` keyword (the helper form, not the modifier form), where trivial means:

1. It's a co-located component (`.hbs` next to `.js`)
2. The JS file has a default export that is native class
3. `{{action "foo"}}` or `(action "foo")` with no extra arguments
4. There is a corresponding `foo()` method defined on the class (not inherited, etc)

There are more usages that is slightly more involved (with arguments, etc) that we can deal with, but this PR seems big enough so I just included the easiest cases here.

To aid review, each file is converted in an individual commit, and the matching method is temporary annotated with `@__action__` instead of the normal `@action`. This forces a git diff when it is already annotated as `@action`.

* DEV: {{action}} -> @action admin-penalty-post-action.hbs
* DEV: {{action}} -> @action admin-report.hbs
* DEV: {{action}} -> @action admin-watched-word.hbs
* DEV: {{action}} -> @action emoji-value-list.hbs
* DEV: {{action}} -> @action bool.hbs
* DEV: {{action}} -> @action category.hbs
* DEV: {{action}} -> @action secret-value-list.hbs
* DEV: {{action}} -> @action category-list.hbs
* DEV: {{action}} -> @action color.hbs
* DEV: {{action}} -> @action compact-list.hbs
* DEV: {{action}} -> @action group-list.hbs
* DEV: {{action}} -> @action host-list.hbs
* DEV: {{action}} -> @action named-list.hbs
* DEV: {{action}} -> @action simple-list.hbs
* DEV: {{action}} -> @action tag-group-list.hbs
* DEV: {{action}} -> @action tag-list.hbs
* DEV: {{action}} -> @action value-list.hbs
* DEV: {{action}} -> @action watched-word-form.hbs
* DEV: {{action}} -> @action composer-messages.hbs
* DEV: {{action}} -> @action section.hbs
* DEV: {{action}} -> @action user-status-picker.hbs
* DEV: cleanup @__action__ -> @action
* UX: separate invite-signup styling

* UX: invite page centering

* remove old invites-show css

* UX: invite signup page – mobile

* remove class references in general file

* add styling for instructions
…#24274)

Reports can have the radar type, which will get rendered by the `admin-report-radar` component.
These have been broken since fd07c94
because watched words were not correctly transformed to regexps.
This partially reverts the changes.
* DEV: Adds a GitHub workflow to check target branch

Adds a GitHub workflow to check that the target branch for PRs in the
discourse-private-mirror repo aren't set to the tests-passed branch.

* Rename workflow
dependabot bot and others added 26 commits November 22, 2023 23:38
Bumps the types group in /app/assets/javascripts with 1 update: [@types/jquery](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/jquery).

- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/jquery)

---
updated-dependencies:
- dependency-name: "@types/jquery"
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: types
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Followup to e37fb30

Some plugins like discourse-ai and discourse-saml do not
nicely change from kebab-case to Title Case (e.g. Ai, Saml),
and anyway this method of getting the plugin name is not
translated either.

Better to use the plugin setting category if it exists,
since that is written by a human and is translated.
…#24513)

Why this change?

Similar to d0117ff, `plugins:update_all` spends most of its time waiting
on the network. On my local machine, this takes up to 2 mins when I have
all the official plugins installed. On a 32 cores machine, the total
time is cut down to 4 seconds.

What does this change do?

1. Move the logic in the `plugin:update` Rake task into a method.
2. Updates the `plugin:update` and `plugin:update_all` to rely on the
   new method.
3. Wraps the method call to update a plugin in `plugin:update_all` in a
   `Concurrent::Promise`

This change also adds the `--quiet` option to the `git pull` option
since the `git pull` output is just noise for 99% of the time.
…iscourse#24521)

This allows us to use that class without loading Rails, e.g. in imports (converters).
…iscourse#24522)

Why this change?

This regressed in dec68d7 where the
commit assumes that plugin gems are always installed when the
`plugin:install_all_gems` Rake task is ran as it would run the our Rails
initializers which activates plugins and install the gems. However, this
assumption only holds true when the `LOAD_PLUGINS` is present and set to
`1`.

What does this change do?

This commit changes the `plugin:install_all_gems` to load the Rails
environment with `LOAD_PLUGINS` set to `1` such that the plugin gems
will be installed as part of our initialization process for the app.

The commit also removes the `plugin:install_gems` Rake task which is
currently a noop and does not seem to be used anywhere..
This change converts the `email_in_min_trust` site setting to
`email_in_allowed_groups`.

See: https://meta.discourse.org/t/283408

- Hides the old setting
- Adds the new site setting
- Add a deprecation warning
- Updates to use the new setting
- Adds a migration to fill in the new setting if the old setting was
  changed
- Adds an entry to the site_setting.keywords section
- Updates tests to account for the new change

After a couple of months we will remove the
`email_in_min_trust` setting entirely.

Internal ref: /t/115696
…n gem (discourse#24522)" (discourse#24524)

This breaks the `plugin:install_all_gems` Rake task when used before
Redis is running. Need to go back to the drawing board.

This reverts commit 189aa5f.
…e#24525)

We were seeing lots of deadlocks deploying this migration. This improves
the situation in 2 ways.

1. ddl transaction is avoided, so we hold locks for far shorter times
2. we operate in chunks of a maximum of 100_000 posts (though it is heavily filtered down)

* improve code so it is clearer
Bumps [aws-eventstream](https://github.com/aws/aws-sdk-ruby) from 1.2.0 to 1.3.0.
- [Release notes](https://github.com/aws/aws-sdk-ruby/releases)
- [Changelog](https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-eventstream/CHANGELOG.md)
- [Commits](aws/aws-sdk-ruby@1.2.0...1.3.0)

---
updated-dependencies:
- dependency-name: aws-eventstream
  dependency-type: indirect
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
So it actually shows up in CI (in a form other than `[Object object]`)
Chat will now check for the state of `SiteSetting.private_email` when sending the summary, when enabled, the mail will not display user information, channel information other than the ID and no message information, only the count of messages.
+ native classes
+ tracked properties
- Ember.Object
- Ember.Evented
- observers
- mixins
- computed/discourseComputed

Also removes unused wizard infrastructure for warnings. It appears
that once upon on time, either the server can generate warnings,
or some client code can generate them, which requires an extra 
confirmation from the user before they can continue to the next step.

This code is not tested and appears unused and defunct. Nothing
generates such warning and the server does not serialize them.

Extracted from discourse#23678
This code has gone through a lot of refactors and repurposing, and
things have become more complicated and confusing than they needed
to be.

At present, the Wizard goes through a few "required" steps for the
basics, then shows a "ready" screen directing the user to "jump in"
but have an option to continue on for optional screens, eventually
ending on the "corporate" screen which is the final step.

On desktop, there are two groups of buttons –

1. on the left side, it shows a back button except for the first
   screen

2. on the right side, there is a always a primary button (either
   "next" or "jump in"), and then to its left, there is sometimes
   a secondary action styled as a link (either "configure more" or
   "exit setup")

On mobile, these buttons are stacked, with the back button sitting
at the bottom, if present.

In table form:

Step        Back Button?     Primary Action      Secondary Action
------------------------------------------------------------------
First            No               Next                  N/A
------------------------------------------------------------------
...             Yes               Next                  N/A
------------------------------------------------------------------
Ready           Yes              Jump In          Configure More
------------------------------------------------------------------
...             Yes               Next              Exit Setup
------------------------------------------------------------------
Last            Yes              Jump In                N/A
------------------------------------------------------------------

Back Button: without saving, go back to the last page
Next Button: save, and if successful, go to the next page
Configure More: re-skinned next button
Exit Setup: without saving, go to the home page ("finish")
Jump In: on the "ready" page, it exits the setup ("finish"), on the
last page, it saves, and if successful, go to the home page

This commit re-aligns the code to reflect this reality without the
unnecessary indirections leftover from previous refactors.
Move field components into subfolders, remove dependency on string
resolution with the runtime resolver.
* Move Wizard back into main app, remove Wizard addon
* Remove Wizard-related resolver or build hacks
* Add a `app/static` folder in the app, add it to `staticAppPaths`
* Install and enable `@embroider/router`
* Add "wizard" to `splitAtRoutes`

In a fully optimized Embroider app, route-based code splitting more
or less Just Work™ – install `@embroider/router`, subclass from it,
configure which routes you want to split and that's about it.

However, our app is not "fully optimized", by which I mean we are
not able to turn on all the `static*` flags.

In Embroider, "static" means "statically analyzable". Specifically
it means that all inter-dependencies between modules (files) are
explicitly expressed as `import`s, as opposed to `{{i18n ...}}`
magically means "look for the default export in app/helpers/i18n.js"
or something even more dynamic with the resolver.

Without turning on those flags, Embroider behaves conservatively,
slurps up all `app` files eagerly into the primary bundle/chunks.
So, while you _could_ turn on route-based code splitting, there
won't be much to split.

The commits leading up to this involves a bunch of refactors and
cleanups that 1) works perfectly fine in the classic build, 2) are
good and useful in their own right, but also 3) re-arranged thigns
such that most dependencies are now explicit.

With those in place, I was able to move all the wizard code into
the "app/static" folder. Embrodier does not eagerly pull things from
this folder into any bundle, unless something explicitly "asks" for
them via `imports`. Conversely, thigns from this folder are not
registered with the resolver and are not added to the `loader.js`
registry.

In conjunction with route-based code splitting, we now have the
ability to split out islands of on-demand functionalities from the
main app bundle.

When you split a route in Embroider, it automatically creates a
bundle/entrypoint with the relevant routes/templates/controllers
matching that route prefix. Anything they import will be added to
the bundle as well, assuming they are not alreay in the main app
bundle, which is where the "app/static" folder comes into play.

The "app/static" folder name is not special. It is configured in
ember-cli-build.js. Alternatively, we could have left everything
in their normal locations, and add more fine-grained paths to the
`staticAppPaths` array. I just thought it would be easy to manage
and scale, and less error-prone to do it this way.

Note that putting things in `app/static` does not guarentee that
it would not be part of the main app bundle. For example, if we
were to add an `import ... from "app/static/wizard/...";` in a
main bundle file (say, `app.js`), then that chunk of the module
graph would be pulled in. (Consider using `await import(...)`?)

Overtime, we can build better tooling (e.g. lint rules and babel
macros to make things less repetitive) as we expand the use of
this pattern, but this is a start.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.