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

Remove the "Getting Started" Section from the Config #16033

Conversation

juliannatetreault
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

This PR removes the "Getting Started" section, and the code related to this section, from /admin/customization/config. With the recent changes to the Creator Onboarding flow, this section is no longer useful.

Related Tickets & Documents

Relates to PR #15969 and more specifically, to this comment.

QA Instructions, Screenshots, Recordings

To QA this PR, please navigate to /admin/customization/config to ensure that everything still works as expected. Additionally, all checks should be green. ♻️

Before:

Screen Shot 2022-01-10 at 11 45 32 AM

After:

Screen Shot 2022-01-10 at 11 32 35 AM

UI accessibility concerns?

N/A

Added/updated tests?

  • Yes in the form of removing any related tests to the code that is being removed. ✂️
  • No, and this is why: please replace this line with details on why tests
    have not been included
  • I need help with writing tests

[Forem core team only] How will this change be communicated?

Will this PR introduce a change that impacts Forem members or creators, the
development process, or any of our internal teams? If so, please note how you
will share this change with the people who need to know about it.

  • I've updated the Developer Docs or
    Storybook (for Crayons components)
  • This PR changes the Forem platform and our documentation needs to be
    updated. I have filled out the
    Changes Requested. The issue for this request can be found here!
    issue template so Community Success can help update the Admin Docs
    appropriately.
  • I've updated the README or added inline documentation
  • I've added an entry to
    CHANGELOG.md
  • I will share this change in a Changelog
    or in a forem.dev post
  • I will share this change internally with the appropriate teams
  • I'm not sure how best to communicate this change and need help: what is the best way to communicate this change to the broader team and community?
  • This change does not need to be communicated, and this is why not: please
    replace this line with details on why this change doesn't need to be
    shared

[optional] Are there any post deployment tasks we need to perform?

N/A

[optional] What gif best describes this PR or how it makes you feel?

alt_text

@pr-triage pr-triage bot added the PR: draft bot applied label for PR's that are a work in progress label Jan 10, 2022
@juliannatetreault
Copy link
Contributor Author

With this removal, should we also remove the red "Required" tags next to the "community name," "community description," "suggested users," and "suggested tags" fields? While they no longer appear in the "Get Started" section (since that has been removed 😄 ) they do appear next to the individual fields throughout the Config (for example, under "Community Content "-> "Community Name"). The "Required" tags do not align with the required fields during the Creator Onboarding flow, rather they appear next to any settings that have been deemed "Mandatory"--I feel as though this is slightly confusing. To add to the confusion, while there is a red "Required" tag next to "suggested tags" and "suggested users," I'm able to clear out both fields and leave them empty without running into any errors.

Thoughts, @Ridhwana?

@Ridhwana
Copy link
Contributor

With this removal, should we also remove the red "Required" tags next to the "community name," "community description," "suggested users," and "suggested tags" fields?

Nice catch, yes please, let's do that.

To add to the confusion, while there is a red "Required" tag next to "suggested tags" and "suggested users," I'm able to clear out both fields and leave them empty without running into any errors.

Ooh, this is weird! but I guess something we can leave a mystery now that we're removing the "Required" 😅

Copy link
Contributor

@Ridhwana Ridhwana left a comment

Choose a reason for hiding this comment

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

Thanks for this Julianna - I've left some comments and insights into the failing test. Let me know if you have any questions :)

@juliannatetreault juliannatetreault changed the title [Not Ready for Review]: Remove the "Getting Started" Section from the Config Remove the "Getting Started" Section from the Config Jan 19, 2022
@juliannatetreault juliannatetreault requested review from nickytonline and Ridhwana and removed request for Ridhwana January 19, 2022 19:51
@juliannatetreault juliannatetreault marked this pull request as ready for review January 19, 2022 19:51
@juliannatetreault juliannatetreault requested review from a team January 19, 2022 19:51
@pr-triage pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: draft bot applied label for PR's that are a work in progress labels Jan 19, 2022
@juliannatetreault juliannatetreault removed request for a team January 19, 2022 19:52
Copy link
Contributor

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

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

This looks great and it appears you've addressed @Ridhwana's feedback. 🚀

Copy link
Contributor

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

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

🚀

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Jan 19, 2022
Copy link
Contributor

@Ridhwana Ridhwana left a comment

Choose a reason for hiding this comment

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

Looks great 🚀, thanks for addressing the feedback @juliannatetreault!

@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Jan 20, 2022
@pr-triage pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: reviewed-approved bot applied label for PR's where reviewer approves changes labels Jan 20, 2022
@juliannatetreault juliannatetreault merged commit 615137a into main Jan 20, 2022
@juliannatetreault juliannatetreault deleted the juliannatetreault/remove-get-started-section-from-config branch January 20, 2022 14:31
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: unreviewed bot applied label for PR's with no review labels Jan 20, 2022
jeremyf added a commit that referenced this pull request Jan 21, 2022
…user

* main: (36 commits)
  Stripping tags from "tags.short_summary" column (#16248)
  Add host tag in ForemStatsClient (#16237)
  Configured the repository to handle line endings (#16234)
  Removed font-awesome from the admin section (#16239)
  fix (#16244)
  remove assignment of css variable to undefined (#16243)
  Add new tag autocomplete to editor (#16025)
  Color tokens (#16107)
  ListingsToolkit refactoring (#16184)
  Expanding the range of followed tags we show (#16213)
  Let override of ToS and CoC show in onboarding (#16217)
  Bump stripe from 5.42.0 to 5.43.0 (#16233)
  Bump flipper-active_record from 0.23.0 to 0.23.1 (#16231)
  Bump flipper-ui from 0.23.0 to 0.23.1 (#16232)
  Bump flipper-active_support_cache_store from 0.23.0 to 0.23.1 (#16230)
  Bump js-routes from 2.2.0 to 2.2.1 (#16229)
  Adds suggested_tags to Onboarding section in Config (#16228)
  Constantize broadcast messages (#16219)
  app/mailers i18n (#16191)
  Remove the "Getting Started" Section from the Config (#16033)
  ...
headline-design pushed a commit to headline-design/forem-1 that referenced this pull request Mar 11, 2022
* Removes the Getting Started section and related code from the config

* Removes the admin_manages_configuration_spec.rb

* Removes "Required" tags from Config

* Reverts removal of activateMissingKeysModal

* Removes mandatory.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants