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.

Sign up for GitHub

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

Jump to bottom

Modernize built-in components (part 2) #707

Merged
merged 4 commits into from
Jan 22, 2021
Merged

Modernize built-in components (part 2) #707

merged 4 commits into from
Jan 22, 2021

Conversation

chancancode
Copy link
Member

@chancancode chancancode commented Jan 15, 2021 •
edited
Loading

@chancancode chancancode added T-components T-framework RFCs that impact the ember.js library T-templates labels Jan 15, 2021
@chancancode chancancode added this to the 4.0 milestone Jan 15, 2021
@ijlee2 ijlee2 mentioned this pull request Jan 15, 2021
Merged
37 tasks
@rwjblue
Copy link
Member

rwjblue commented Jan 15, 2021

We discussed this at todays core team meeting, and are moving it into final comment period.

ro0gr
ro0gr reviewed Jan 20, 2021
View reviewed changes
text/0707-modernize-built-in-components-2.md
element for navigation is not recommended as it creates issues with assistive
technologies. Remove this argument to use the default <a> element. In the rare
cases that calls for using a different element, refactor to use the router
service inside a custom event handler instead.
Copy link

@ro0gr ro0gr Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Does it make sense to have a link here to appropriate section of the RFC, to provide more context, and detailed description like it is in the RFC.

This "no longer recommended" feels like a soft form of deprecations. But deprecations typically have their link.

@rwjblue rwjblue merged commit f309e67 into master Jan 22, 2021
@rwjblue rwjblue deleted the modernizing-built-in-components branch January 22, 2021 19:13
@BryanCrotaz
Copy link

Rendered link is a 404?

@chancancode
Copy link
Member Author

Rendered link is a 404?

Fixed now. Generally this is a problem after merging the RFCs because while still in PRs, the rendered links points to the branch, but once it's merged the branch is deleted and it lives on the master branch instead, so the link would have to be updated

@chriskrycho chriskrycho mentioned this pull request Sep 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ro0gr ro0gr ro0gr left review comments

@CvX CvX CvX left review comments

Assignees
No one assigned
Labels
Final Comment Period T-components T-framework RFCs that impact the ember.js library T-templates
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@chancancode @rwjblue @BryanCrotaz @CvX @ro0gr