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

Change footer's todo item filter anchor tag components to button tag #2183

Open
krapie opened this issue Jan 6, 2023 · 0 comments
Open

Change footer's todo item filter anchor tag components to button tag #2183

krapie opened this issue Jan 6, 2023 · 0 comments

Comments

@krapie
Copy link

krapie commented Jan 6, 2023

I suggest change anchor tag to button tag in footer's todo item filter list.

There are 2 reasons for this suggestion.

  1. Anchor tag <a> is meant for routing/redirecting to other pages. In the other hand, button tag <button> is meant for all actions that is not routing/redirecting. This is recommended for sematic web. Since todo item filter is meant for filtering todo items and not routing, components for filter should be button instead for anchor.
  2. Anchor tag can cause unexpected routing in some conditions. For example, anchor tags as button in iframe will redirect to iframe's parent page. In https://yorkie.dev/examples/todomvc example, when you click filtering options (All, Active, Completed), you will be redirected to iframe's parent url, which is https://yorkie.dev/#. Below is demo for demonstrating this problem.
    2023-01-06 18-29-32

For this two reasons, I suggest change anchor tag to button tag in footer's todo item filter list.
Code for React/Typescript will look like:

export default function Footer(props: FooterProps) {
  const {
    activeCount,
    completedCount,
    filter: selectedFilter,
    onClearCompleted,
    onShow
  } = props;
  return (
    <footer className="footer">
      <span className="todo-count">
        <strong>{activeCount || 'No'}</strong>
        &nbsp;{activeCount === 1 ? 'item' : 'items'} left
      </span>
      <ul className="filters">
        {
          ['SHOW_ALL', 'SHOW_ACTIVE', 'SHOW_COMPLETED'].map((filter) => (
            <li key={filter}>
              <button
                type="button"
                className={classnames({ selected: filter === selectedFilter })}
                style={{ cursor: 'pointer' }}
                onClick={() => onShow(filter)}
              >
                {FILTER_TITLES[filter]}
              </button>
            </li>
          ))
        }
      </ul>
      {!!completedCount && (
        <button type="button" className="clear-completed" onClick={onClearCompleted}>
          Clear completed
        </button>
      )}
    </footer>
  );
}

To keep appearance same as before, this CSS should be applied.

.filters li button {
  color: inherit;
  margin: 0px 3px 0px 3px;
  padding: 0px 3px 0px 3px;
  text-decoration: none;
  border: 1px solid transparent;
  border-radius: 3px;
}

.filters li button:hover {
  border-color: #DB7676;
}

.filters li button.selected {
  border-color: #CE4646;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant