Skip to content

Commit

Permalink
DEV 1483: Fix bug in how error messages are displayed (#59)
Browse files Browse the repository at this point in the history
* bugfix: error messages hidden when prefix is 0:

* show error messages on snake highlight instead of tiny icon button

* highlight selected avatar differently from hover state

* update README with some tips and warnings

* update ci.yml to use specific node version and reduce duplication
  • Loading branch information
robbles authored Jun 29, 2022
1 parent f857be9 commit 2a25f91
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 58 deletions.
43 changes: 16 additions & 27 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,24 @@ on: [push, pull_request]
# for testing purposes. Will focus on main branch after thia workflow is integrated

jobs:

lint:
build:
runs-on: ubuntu-latest

strategy:
matrix:
node-version: [10.19]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/
#
steps:
- name: Checkout latest source
uses: actions/checkout@v2

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v2
with:
node-version: ${{ matrix.node-version }}
cache: "npm"

- name: Install package dependencies
run: npm ci

Expand All @@ -20,15 +30,10 @@ jobs:
env:
CI: true

build:
runs-on: ubuntu-latest

steps:
- name: Checkout latest source
uses: actions/checkout@v2

- name: Install package dependencies
run: npm ci
- name: Test
run: npm test
env:
CI: true

- name: Build react app (board)
run: npm run build
Expand All @@ -40,19 +45,3 @@ jobs:
with:
name: board artifacts
path: public/

test:
runs-on: ubuntu-latest
#needs: [lint, build]

steps:
- name: Checkout latest source
uses: actions/checkout@v2

- name: Install package dependencies
run: npm ci

- name: Test
run: npm test
env:
CI: true
12 changes: 10 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ This project follows most React conventions and tools described in the react doc
## Development

### Configure
Create a `.env.local` file at the root of the project to set the host variable. `localhost` is the default but will not work with the CORS policy for snake part svg files. `127.0.0.1` is whitelisted for CORS.

This project requires Node 10.19. The dependencies may fail to install on newer versions.

Create a `.env.local` file at the root of the project to set the host that the local board URL serves from. `localhost` is the default but will not work with the CORS policy for snake part svg files. `127.0.0.1` is whitelisted for CORS.

```shell
# File: /.env.local
Expand All @@ -17,10 +20,15 @@ HOST=127.0.0.1

### Install & Run
```shell
npm i
# Installs dependencies from package-lock.json
npm ci

# Starts a server on http://127.0.0.1:3000 and opens it in your default browser
npm start
```

**NOTE**: The SVG assets have a CORS policy that only allows specific hostnames. If you change the local port used by the server, the board might not work.

## Production

```shell
Expand Down
17 changes: 2 additions & 15 deletions src/components/avatar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ class Avatar extends React.Component {
this.state = {
showError: false
};

this.toggleErrorMessage = this.toggleErrorMessage.bind(this);
}
render() {
return (
Expand All @@ -86,10 +84,7 @@ class Avatar extends React.Component {
</div>
<AuthorWrapper>
<Author>by {this.props.snake.author}</Author>
<StatusIndicator
errorMessage={this.props.snake.error}
clickHandler={e => this.toggleErrorMessage(e)}
/>
<StatusIndicator errorMessage={this.props.snake.error} />
<Latency latency={this.props.snake.latency}>
{this.props.snake.latency} ms
</Latency>
Expand All @@ -109,19 +104,11 @@ class Avatar extends React.Component {
</HealthBarWrapper>
)}
<ErrorMessage
error={this.state.showError ? this.props.snake.error : ""}
error={this.props.highlighted ? this.props.snake.error : ""}
/>
</AvatarWrapper>
);
}

toggleErrorMessage(e) {
if (e.target !== e.currentTarget) {
this.setState({
showError: !this.state.showError
});
}
}
}

export default Avatar;
10 changes: 8 additions & 2 deletions src/components/scoreboard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,17 @@ const Wrapper = styled("div")`
const AvatarWrapper = styled("div")`
padding: 0.75rem;
transition: background-color 0.2s, box-shadow 0.2s;
border: 2px solid ${props => (props.highlighted ? "#888" : "transparent")};
border: 2px solid
${props =>
props.highlighted ? colors.avatarHighlightedBorder : "transparent"};
&:hover {
background-color: ${props =>
props.theme === themes.dark ? colors.purple : colors.light};
cursor: pointer;
border-color: #888;
border-color: ${props =>
props.highlighted
? colors.avatarHighlightedBorder
: colors.avatarHoverBorder};
}
`;

Expand Down Expand Up @@ -91,6 +96,7 @@ class Scoreboard extends React.Component {
snake={snake}
key={"avatar" + i}
theme={this.props.theme}
highlighted={highlightedSnake === snake._id}
/>
</AvatarWrapper>
))
Expand Down
11 changes: 5 additions & 6 deletions src/components/scoreboard/error-message/ErrorMessage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,18 @@ const errorColor = {
};

function getIcon(code) {
return code ? "error" : "";
return "error";
}

const ErrorMessage = props => {
let code, message;

if (props.error) {
[code, message] = props.error.toString().split(":");
}

if (!Number(code)) {
if (props.error === "") {
return "";
}

[code, message] = props.error.toString().split(":");

return (
<div className="error-message" style={errorColor}>
<span className="material-icons material-icons-inline">
Expand Down
13 changes: 7 additions & 6 deletions src/components/scoreboard/status-indicator/StatusIndicator.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,19 @@ const style = {
};

function getIcon(code) {
return code ? "error" : "warning";
return "error";
}

const StatusIndicator = ({ errorMessage = "", clickHandler = () => {} }) => {
const StatusIndicator = ({ errorMessage }) => {
if (errorMessage === "") {
return "";
}

// until we can refactor Avatar and not pass the whole error object
const [code] = errorMessage.toString().split(":");

if (!Number(code)) {
return "";
}
return (
<div style={style} onClick={clickHandler}>
<div style={style}>
<span className="material-icons material-icons-inline">
{getIcon(code)}
</span>
Expand Down
2 changes: 2 additions & 0 deletions src/theme/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export const colors = {
healthBarOutline: "#666",
healthBarDeathBackground: "#f1f1f1",
healthBarDeathBackgroundDark: "#59526b",
avatarHoverBorder: "#888",
avatarHighlightedBorder: "#6f42c1",

// Misc
lightText: "#efefef",
Expand Down

0 comments on commit 2a25f91

Please sign in to comment.