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

fix: metrics consent prompt location and styling #353

Merged
merged 3 commits into from
Jan 4, 2023
Merged

fix: metrics consent prompt location and styling #353

merged 3 commits into from
Jan 4, 2023

Conversation

0xDanomite
Copy link
Contributor

Fixes #341 and addresses #340

  • Moved cookie toggle button to lower left
  • Updated metric consent prompt to be fixed at bottom of page
  • Updated styles of metric consent prompt (interim fix until new modal updates are added in feat: finalize metrics modal and icon #340)

Screen Shot 2022-12-19 at 2 02 47 PM

Screen.Recording.2022-12-19.at.2.03.50.PM.mov

@0xDanomite 0xDanomite requested a review from a team as a code owner December 20, 2022 01:26
@welcome
Copy link

welcome bot commented Dec 20, 2022

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions
    and stakeholders linked/mentioned.

  • Your contribution itself is clear (code comments, self-review for the
    rest) and in its best form. Follow the code contribution
    guidelines
    if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on
    any missing things and potentially assigning a reviewer for high
    priority items.

  • The PR gets reviews, discussed and approvals as needed.

  • The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

whizzzkid
whizzzkid reviewed Dec 20, 2022
View reviewed changes
Copy link
Contributor

@whizzzkid whizzzkid left a comment

Choose a reason for hiding this comment

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

I'm seeing a bunch of weird behaviours on smaller screens

Screen.Recording.2022-12-19.at.6.38.35.PM.mov

Screenshot 2022-12-19 at 6 37 45 PM

Screenshot 2022-12-19 at 6 36 33 PM

Screen.Recording.2022-12-19.at.6.34.53.PM.mov
Screen.Recording.2022-12-19.at.6.34.35.PM.mov

SgtPooki
SgtPooki requested changes Dec 20, 2022
View reviewed changes
src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/styles.css Outdated Show resolved Hide resolved
@SgtPooki
Copy link
Member

SgtPooki commented Dec 20, 2022 •
edited
Loading

we should make sure that the banner changes aren't doing new breakages on mobile, but not about fixing any existing issues in this PR (we should open issues for them to address later). We should fix:

  1. the scrolling issue called out by @whizzzkid in Screen.Recording.2022-12-19.at.6.34.35.PM.mov where the allow/decline buttons are not visible. when scrolling up/down.
  2. text not being fully visible in footer.
  3. spacing being too large in footer on smaller screens

@0xDanomite
Copy link
Contributor Author

I'm seeing a bunch of weird behaviours on smaller screens

@whizzzkid I have updated the styling to address the issues you pointed out.

@whizzzkid
Copy link
Contributor

@djmcquillan I'm still seeing this behaviour:

Screen.Recording.2022-12-21.at.4.36.16.PM.mov

whizzzkid
whizzzkid approved these changes Jan 3, 2023
View reviewed changes
Copy link
Contributor

@whizzzkid whizzzkid left a comment

Choose a reason for hiding this comment

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

lgtm! thanks for fixing this

src/styles.css
Comment on lines +137 to +140
position: fixed;
bottom: 0;
left: 0;
z-index: 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

this should've fixed it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have thought so too! it should have, but firefox was having issue with scroll as you noticed so the results table container needed an overflow setting

src/styles.css
display: flex;
align-items: center;
justify-content: center;
max-width: 1024px;
Copy link
Contributor

Choose a reason for hiding this comment

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

prolly not a bad idea to define min-width too?

@0xDanomite 0xDanomite merged commit e709f2b into master Jan 4, 2023
@0xDanomite 0xDanomite deleted the fix/metrics-consent-prompt/341 branch January 4, 2023 20:27
0xDanomite added a commit that referenced this pull request Jan 7, 2023
@0xDanomite
fix: metrics consent prompt location and styling (#353)
eb4ce2e 
* fix: metrics consent prompmt location and styling

* fix: metrics consent styling updates per cr

* fix: 341 - scroll issues on mobile fix
github-actions bot pushed a commit that referenced this pull request Apr 17, 2023
@semantic-release-bot
chore(release): 1.0.0 [skip ci]
5f2811b 
## 1.0.0 (2023-04-17)

### Features

* /ipns/ check ([#313](#313)) ([10a5c13](10a5c13))
* add countly metrics ([#309](#309)) ([c727202](c727202))
* add cthd.icu ([#294](#294)) ([a2e0102](a2e0102))
* add https://ipfs.czip.it ([#374](#374)) ([f3dde51](f3dde51))
* add https://ipfs.joaoleitao.org ([#323](#323)) ([787f131](787f131))
* add ipfs.1-2.dev ([#169](#169)) ([c764ad6](c764ad6))
* add ipfs.drink.cafe ([#116](#116)) ([cc84899](cc84899))
* add ipfs.jpu.jp ([#348](#348)) ([b52b8c7](b52b8c7))
* add ipfs.litnet.work ([#222](#222)) ([1fd2e68](1fd2e68))
* add ipfs.pinksheep.whizzzkid.dev ([#326](#326)) ([d40a2c1](d40a2c1))
* add ipfs.soul-network.com ([#389](#389)) ([57fe04d](57fe04d))
* add nftstorage.link gateway ([#204](#204)) ([e588108](e588108))
* add Onion Gateway (TOR)  fzdqwfb5ml56oadins5jpuhe6ki6bk33umri35p5kt2tue4fpws5efid.onion ([#212](#212)) ([01ff12f](01ff12f))
* add w3s.link gateway ([#288](#288)) ([000a26f](000a26f))
* country flags ([#96](#96)) ([84a31fe](84a31fe))
* Create CODEOWNERS ([#283](#283)) ([b62b41c](b62b41c))
* Deleted https://ipfs.czip.it ([#393](#393)) ([77d67a4](77d67a4))
* Implementing Trustless Server Checks ([#310](#310)) ([4a2c926](4a2c926))
* improved Origin detection via img tag ([#117](#117)) ([8407e80](8407e80))
* improved origin isolation check ([#148](#148)) ([abd4c1c](abd4c1c))
* Introducing Service Worker For Cache Busting ([#357](#357)) ([0536782](0536782))
* new gateway https://ipfs.tayfundogdas.me/ipfs ([#321](#321)) ([9d5b552](9d5b552))
* remove ipfs.foxgirl.dev ([#155](#155)) ([15fd028](15fd028))
* remove smartsignature.io ([#146](#146)) ([77b45b6](77b45b6))
* subdomain gateways and Origin isolation check ([#78](#78)) ([afcbffa](afcbffa))
* update geoip dataset (2020-10-13) ([4187738](4187738))
* update geoip dataset (2020-10-13) ([#115](#115)) ([782b66b](782b66b))
* use typescript ([#194](#194)) ([10958e6](10958e6))

### Bug Fixes

* ⏪ Reverting [#323](#323): ipfs.joaoleitao.org ([#394](#394)) ([b5bb34c](b5bb34c))
* **ci:** add empty commit to fix lint checks on master ([3ae6aa0](3ae6aa0))
* **ci:** skip test if no code changed ([#210](#210)) ([7d6d628](7d6d628))
* cleanup entries missing DNS A record ([#180](#180)) ([2b7ad30](2b7ad30))
* do not redirect IPNS checks ([#325](#325)) ([79bb51d](79bb51d))
* flag column and new ipfs-geoip dataset ([#319](#319)) ([f5fc723](f5fc723))
* metrics consent prompt location and styling ([#353](#353)) ([e709f2b](e709f2b))
* npm start should work without prior cmds ([#307](#307)) ([7ebe2e5](7ebe2e5))
* opt-out from redirects done by browser extension ([6dd5f51](6dd5f51))
* origin typo ([#200](#200)) ([d198abb](d198abb))
* **origin:** confirm paths redirect to subdomain ([#156](#156)) ([b837a35](b837a35))
* remove heart ([#332](#332)) ([f61ec84](f61ec84))
* update ipfs.ivoputzer.xyz gateway entry ([#152](#152)) ([4b760d9](4b760d9))
* update metrics collection banner to modal with management toggle settings ([#373](#373)) ([d925b36](d925b36))
* update redirect opt-out symbol to final version ([efd5dbf](efd5dbf))

### Trivial Changes

* **deps-dev:** bump aegir from 36.2.3 to 37.5.5 ([#305](#305)) ([1d62fc3](1d62fc3))
* **deps-dev:** bump aegir from 37.5.5 to 37.5.6 ([#316](#316)) ([d3cd9bd](d3cd9bd))
* **deps-dev:** bump browserslist from 4.19.3 to 4.21.4 ([#295](#295)) ([7850071](7850071))
* **deps-dev:** bump eslint-config-ipfs from 2.1.0 to 3.1.1 ([#300](#300)) ([f1bce91](f1bce91))
* **deps-dev:** bump eslint-config-ipfs from 3.1.1 to 3.1.2 ([#315](#315)) ([0506c19](0506c19))
* **deps-dev:** bump ipfs from 0.62.1 to 0.64.2 ([#296](#296)) ([d47e503](d47e503))
* **deps-dev:** bump ipfs from 0.64.2 to 0.65.0 ([#322](#322)) ([b400349](b400349))
* **deps-dev:** bump typescript from 4.6.2 to 4.8.3 ([#293](#293)) ([c56afa1](c56afa1))
* **deps-dev:** bump typescript from 4.8.3 to 4.8.4 ([#304](#304)) ([899b4fc](899b4fc))
* **deps:** bump @dutu/rate-limiter from v1.3.0 to v1.3.1 ([#299](#299)) ([5e598e8](5e598e8))
* **deps:** bump aegir from 36.1.3 to 36.2.3 ([#202](#202)) ([8fa5851](8fa5851))
* **deps:** bump jpeg-js from 0.4.3 to 0.4.4 ([#253](#253)) ([65f99f3](65f99f3))
* **deps:** ipfs-http-client@58.0.1 ([#308](#308)) ([1bcda7c](1bcda7c))
* improve submission/PR info ([#119](#119)) ([a238f3f](a238f3f))
* improved security notes ([#151](#151)) ([5893f35](5893f35)), closes [#148](#148) [/github.com//pull/151#issuecomment-857193370](https://github.com/ipfs//github.com/ipfs/public-gateway-checker/pull/151/issues/issuecomment-857193370)
* ipfs-geoip v5 ([0d8091e](0d8091e))
* ipfs-geoip@8.0.0 ([c4e8180](c4e8180))
* readme cleanup ([798777e](798777e))
* remove dead hostnames ([#280](#280)) ([e861280](e861280))
* remove expired domains ([#179](#179)) ([16c9985](16c9985))
* remove ipfs-zod.tv ([#234](#234)) ([6f79a80](6f79a80))
* removed birds-are-nice.me ([#173](#173)) ([ff2e05c](ff2e05c)), closes [#172](#172)
* removing my gateway for now ([#335](#335)) ([cf61e68](cf61e68))
* style formatting and linting fixes ([#366](#366)) ([a81d48b](a81d48b))
* Update .github/workflows/stale.yml [skip ci] ([5fc4a68](5fc4a68))
* update readme with link to fleek ([#337](#337)) ([3dc5dbe](3dc5dbe))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whizzzkid whizzzkid whizzzkid approved these changes

@SgtPooki SgtPooki SgtPooki approved these changes

@juliaxbow juliaxbow Awaiting requested review from juliaxbow

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move cookie prompt to the bottom
3 participants
@0xDanomite @SgtPooki @whizzzkid