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

[Announcement] Adjust block id for displaying dynamic names #327

Merged
merged 6 commits into from
Aug 9, 2021

Conversation

melissaperreault
Copy link
Contributor

@melissaperreault melissaperreault commented Aug 5, 2021

Why are these changes introduced?

Currently Announcement bar have static names and the experience for merchants isn't great.

Related to this the recent update https://github.com/Shopify/online-store-web/issues/10953#issuecomment-879094000 we need to change the id for heading rather than text

I tested on the editor and seems to work as expected. Unsure though if I have to change something else?

@melissaperreault melissaperreault changed the title Adjust id for displaying dynamic names [Announcement] Adjust block id for displaying dynamic names Aug 5, 2021
@melissaperreault melissaperreault added the Category: Bug Something isn't working label Aug 5, 2021
ludoboludo
ludoboludo previously approved these changes Aug 5, 2021
Copy link
Contributor

@ludoboludo ludoboludo left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

tauthomas01
tauthomas01 previously approved these changes Aug 5, 2021
Copy link
Contributor

@tauthomas01 tauthomas01 left a comment

Choose a reason for hiding this comment

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

Looks good!

@melissaperreault
Copy link
Contributor Author

@ludoboludo @tauthomas01 I am ready for a final review I believe, thanks again a bunch!

Copy link
Contributor

@tauthomas01 tauthomas01 left a comment

Choose a reason for hiding this comment

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

Good to go

Copy link
Contributor

@ludoboludo ludoboludo left a comment

Choose a reason for hiding this comment

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

🚀

@melissaperreault melissaperreault merged commit 9451a31 into main Aug 9, 2021
@melissaperreault melissaperreault deleted the announcement-change branch August 9, 2021 17:20
maplerock added a commit to fellowsmedia/nsra_shopify_2021 that referenced this pull request Aug 12, 2021
* main: (23 commits)
  Cart content update (Shopify#370)
  Footer spacing and alignment adjustments (Shopify#369)
  Product Template UI polish updates (Shopify#219)
  footer ui updates (Shopify#318)
  Fix cart improvements empty state (Shopify#319)
  [Announcement] Adjust block id for displaying dynamic names (Shopify#327)
  Update translations: buyer (Shopify#329)
  Revert editor setting changes (Shopify#328)
  Update translations (Shopify#294)
  Fix collection filtering UX (Shopify#268)
  Added page width setting and fixed image quality (Shopify#292)
  Add top border on cart notification when "show separator line" setting is deactivated (Shopify#306)
  movebadge code into base.css (Shopify#313)
  Collage UI bug fixes (Shopify#308)
  Customer account UI polish (Shopify#177)
  Added custom liquid block to product page (Shopify#269)
  Fix disclosure icon (Shopify#310)
  Fix facet price filter label spacing (Shopify#300)
  Fix duplicate search icon when header logo is set to "Top center" (Shopify#252)
  Add card outline setting (Shopify#239)
  ...
@PaulNewton
Copy link

@melissaperreault for posterity when shopifolk reference private initiators can we please please PLEASE get a blurb of what's referenced.
It's either that or maybe mark a task for being backfilled when the public documentation is updated with info related to what initiated the code change.
It's such a meta thing I'm not sure if raising a separate issue fits but the repo is filling quickly with inaccessible private information, internal repos, links, password protected demo stores, figma allusions etc and very little docs or inline comments.
Inscrutable histories do not aid future understanding but it will fuel constant questions.

phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
)

* Adjust id for displaying dynamic names

* fix issue

* testing

* Update from Shopify for theme dawn/announcement-change

* Update from Shopify for theme dawn/announcement-change

* adjust overwrite

Co-authored-by: shopify-online-store[bot] <79544226+shopify-online-store[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug Something isn't working cla-needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants