forked from discourse/discourse
-
Notifications
You must be signed in to change notification settings - Fork 0
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
chancancode
wants to merge
470
commits into
classic-wizardry-components
Choose a base branch
from
embroider-wizardry
base: classic-wizardry-components
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f82e7e0
to
3fef4f6
Compare
2bb3bbb
to
dbe71dd
Compare
893a2a9
to
e7026b4
Compare
dbe71dd
to
e92eb03
Compare
e7026b4
to
3a9dab6
Compare
e92eb03
to
0a062a7
Compare
3a9dab6
to
b5ec3b1
Compare
0a062a7
to
0f55e63
Compare
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>
Co-authored-by: AlexDev <[email protected]>
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
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.
0f55e63
to
60cc6b0
Compare
* 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.
60cc6b0
to
42ef186
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.